Return to Answer

Don't comment the obvious.

Because, you know, it's obvious. People expect something unexpected or tricky there, and it's a letdown when they see that you just wasted their time and concentration.

The greatest danger though is that those useless comments could get out-of-sync with the code, leading to lots of confusion and even more lost time.

The API-documentation belongs to the declaration in the header, not the definition, so users find it without digging in the details.

Consider splitting your code into the public header, the list-implementation, and the test-program. Copy-and-paste is a bad method for reuse.

Don't cast the result of malloc(). And avoid using sizeof(type).
Both are error-prone and violate DRY. See: Do I cast the result of malloc?

Consider using a flexible-array-member (C99) for your struct Node. It allows you to save space or store larger strings without extra allocation.

 typedef struct Node {
     struct Node* next;
     int key;
     char string[];
  } Node;

insert_end() will try to modify the non-existent last node if the list was empty. That's somewhat sub-optimal.

Use a double-pointer or special-case it.

remove_beginning() reads the freed ex-node's memory to find the new first node. That's a bit tardy. Read first, then free.

It also fails to update the last-pointer if the list is now empty.

Why does print_list() fail to print an empty list?

free_list() also commits use-after-free, and its two local variables duplicate each other.

Interesting, why does get_string() ignore the key if the list is exactly one element long?

You are generally ignoring allocation-failure. Whether you abort the program or whatever, handle it!

Consider directing your debug-output to stderr. And only writing it if NDEBUG is not defined.

Using C99 variadic macros:

 #ifndef NDEBUG
     #define DPRINT(...) (void)fprintf(stderr, __VA_ARGS__)
 #else
     #define DPRINT(...) (void)0
 #endif

Used like:

 DPRINT("My debug message: %s", somestring);

If you really want to explicitly return from main() using a preprocessor-constant, use the dedicated EXIT_SUCCESS from <stdlib.h>.
Of course, return 0; is implicit for main() since C99...

Don't comment the obvious.

Because, you know, it's obvious. People expect something unexpected or tricky there, and it's a letdown when they see that you just wasted their time and concentration.

The greatest danger though is that those useless comments could get out-of-sync with the code, leading to lots of confusion and even more lost time.

The API-documentation belongs to the declaration in the header, not the definition, so users find it without digging in the details.

Consider splitting your code into the public header, the list-implementation, and the test-program. Copy-and-paste is a bad method for reuse.

Don't cast the result of malloc(). And avoid using sizeof(type).
Both are error-prone and violate DRY. See: Do I cast the result of malloc?

Consider using a flexible-array-member (C99) in your struct Node for the string. It allows you to save space or store larger strings without extra allocation.

 typedef struct Node {
     struct Node* next;
     int key;
     char string[];
 } Node;

insert_end() will try to modify the non-existent last node if the list was empty. That's somewhat sub-optimal.

Use a double-pointer or special-case it.

remove_beginning() reads the freed ex-node's memory to find the new first node. That's a bit tardy. Read first, then free.

It also fails to update the last-pointer if the list is now empty.

Why does print_list() fail to print an empty list?

free_list() also commits use-after-free, and its two local variables duplicate each other.

Interesting, why does get_string() ignore the key if the list is exactly one element long?

You are generally ignoring allocation-failure. Whether you abort the program or whatever, handle it!

Consider directing your debug-output to stderr. And only writing it if NDEBUG is not defined.

Using C99 variadic macros:

 #ifndef NDEBUG
     #define DPRINT(...) (void)fprintf(stderr, __VA_ARGS__)
 #else
     #define DPRINT(...) (void)0
 #endif

Used like:

 DPRINT("My debug message: %s", somestring);

If you really want to explicitly return from main() using a preprocessor-constant, use the dedicated EXIT_SUCCESS from <stdlib.h>.
Of course, return 0; is implicit for main() since C99...

Don't comment the obvious.

Because, you know, it's obvious. People expect something unexpected or tricky there, and it's a letdown when they see that you just wasted their time and concentration.

The greatest danger though is that those useless comments could get out-of-sync with the code, leading to lots of confusion and even more lost time.

The API-documentation belongs to the declaration in the header, not the definition, so users find it without digging in the details.

Consider splitting your code into the public header, the list-implementation, and the test-program. Copy-and-pate is a bad method for reuse.

Don't cast the result of malloc(). And avoid using sizeof(type).
Both are error-prone and violate DRY. See: Do I cast the result of malloc?

Consider using a flexible-array-member (C99) for your struct Node. It allows you to save space or store larger strings without extra allocation.

 typedef struct Node {
     struct Node* next;
     int key;
     char string[];
  } Node;

insert_end() will try to modify the non-existent last node if the list was empty. That's somewhat sub-optimal.

Use a double-pointer or special-case it.

remove_beginning() reads the freed ex-node's memory to find the new first node. That's a bit tardy. Read first, then free.

It also fails to update the last-pointer if the list is now empty.

Why does print_list() fail to print an empty list?

free_list() also commits use-after-free, and its two local variables duplicate each other.

Interesting, why does get_string() ignore the key if the list is exactly one element long?

You are generally ignoring allocation-failure. Whether you abort the program or whatever, handle it!

Consider directing your debug-output to stderr. And only writing it if NDEBUG is not defined.

Using C99 variadic macros:

 #ifndef NDEBUG
     #define DPRINT(...) (void)fprintf(stderr, __VA_ARGS__)
 #else
     #define DPRINT(...) (void)0
 #endif

Used like:

 DPRINT("My debug message: %s", somestring);

If you really want to explicitly return from main() using a preprocessor-constant, use the dedicated EXIT_SUCCESS from <stdlib.h>.
Of course, return 0; is implicit for main() since C99...

Don't comment the obvious.

Because, you know, it's obvious. People expect something unexpected or tricky there, and it's a letdown when they see that you just wasted their time and concentration.

The greatest danger though is that those useless comments could get out-of-sync with the code, leading to lots of confusion and even more lost time.

The API-documentation belongs to the declaration in the header, not the definition, so users find it without digging in the details.

Consider splitting your code into the public header, the list-implementation, and the test-program. Copy-and-paste is a bad method for reuse.

Don't cast the result of malloc(). And avoid using sizeof(type).
Both are error-prone and violate DRY. See: Do I cast the result of malloc?

Consider using a flexible-array-member (C99) for your struct Node. It allows you to save space or store larger strings without extra allocation.

 typedef struct Node {
     struct Node* next;
     int key;
     char string[];
  } Node;

insert_end() will try to modify the non-existent last node if the list was empty. That's somewhat sub-optimal.

Use a double-pointer or special-case it.

remove_beginning() reads the freed ex-node's memory to find the new first node. That's a bit tardy. Read first, then free.

It also fails to update the last-pointer if the list is now empty.

Why does print_list() fail to print an empty list?

free_list() also commits use-after-free, and its two local variables duplicate each other.

Interesting, why does get_string() ignore the key if the list is exactly one element long?

You are generally ignoring allocation-failure. Whether you abort the program or whatever, handle it!

Consider directing your debug-output to stderr. And only writing it if NDEBUG is not defined.

Using C99 variadic macros:

 #ifndef NDEBUG
     #define DPRINT(...) (void)fprintf(stderr, __VA_ARGS__)
 #else
     #define DPRINT(...) (void)0
 #endif

Used like:

 DPRINT("My debug message: %s", somestring);

If you really want to explicitly return from main() using a preprocessor-constant, use the dedicated EXIT_SUCCESS from <stdlib.h>.
Of course, return 0; is implicit for main() since C99...

Don't comment the obvious.

Because, you know, it's obvious. People expect something unexpected or tricky there, and it's a letdown when they see that you just wasted their time and concentration.

The API-documentation belongs to the declaration in the header, not the definition, so users find it without digging in the details.

Consider splitting your code into the public header, the list-implementation, and the test-program. Copy-and-pate is a bad method for reuse.

Don't cast the result of malloc(). And avoid using sizeof(type).
Both are error-prone and violate DRY. See: Do I cast the result of malloc?

Consider using a flexible-array-member (C99) for your struct Node. It allows you to save space or store larger strings without extra allocation.

 typedef struct Node {
     struct Node* next;
     int key;
     char string[];
  } Node;

insert_end() will try to modify the non-existent last node if the list was empty. That's somewhat sub-optimal.

Use a double-pointer or special-case it.

remove_beginning() reads the freed ex-node's memory to find the new first node. That's a bit tardy. Read first, then free.

It also fails to update the last-pointer if the list is now empty.

Why does print_list() fail to print an empty list?

free_list() also commits use-after-free, and its two local variables duplicate each other.

Interesting, why does get_string() ignore the key if the list is exactly one element long?

You are generally ignoring allocation-failure. Whether you abort the program or whatever, handle it!

Consider directing your debug-output to stderr. And only writing it if NDEBUG is not defined.

Using C99 variadic macros:

 #ifndef NDEBUG
     #define DPRINT(...) (void)fprintf(stderr, __VA_ARGS__)
 #else
     #define DPRINT(...) (void)0
 #endif

Used like:

 DPRINT("My debug message: %s", somestring);

If you really want to explicitly return from main() using a preprocessor-constant, use the dedicated EXIT_SUCCESS from <stdlib.h>.
Of course, return 0; is implicit for main() since C99...

Don't comment the obvious.

Because, you know, it's obvious. People expect something unexpected or tricky there, and it's a letdown when they see that you just wasted their time and concentration.

The greatest danger though is that those useless comments could get out-of-sync with the code, leading to lots of confusion and even more lost time.

The API-documentation belongs to the declaration in the header, not the definition, so users find it without digging in the details.

Consider splitting your code into the public header, the list-implementation, and the test-program. Copy-and-pate is a bad method for reuse.

Don't cast the result of malloc(). And avoid using sizeof(type).
Both are error-prone and violate DRY. See: Do I cast the result of malloc?

Consider using a flexible-array-member (C99) for your struct Node. It allows you to save space or store larger strings without extra allocation.

 typedef struct Node {
     struct Node* next;
     int key;
     char string[];
  } Node;

insert_end() will try to modify the non-existent last node if the list was empty. That's somewhat sub-optimal.

Use a double-pointer or special-case it.

remove_beginning() reads the freed ex-node's memory to find the new first node. That's a bit tardy. Read first, then free.

It also fails to update the last-pointer if the list is now empty.

Why does print_list() fail to print an empty list?

free_list() also commits use-after-free, and its two local variables duplicate each other.

Interesting, why does get_string() ignore the key if the list is exactly one element long?

You are generally ignoring allocation-failure. Whether you abort the program or whatever, handle it!

Consider directing your debug-output to stderr. And only writing it if NDEBUG is not defined.

Using C99 variadic macros:

 #ifndef NDEBUG
     #define DPRINT(...) (void)fprintf(stderr, __VA_ARGS__)
 #else
     #define DPRINT(...) (void)0
 #endif

Used like:

 DPRINT("My debug message: %s", somestring);

If you really want to explicitly return from main() using a preprocessor-constant, use the dedicated EXIT_SUCCESS from <stdlib.h>.
Of course, return 0; is implicit for main() since C99...