I have a code that I'm trying to do kinda playlist of songs. They'll be on a struct connected by linked lists, which contain the song name and time. In the program, there is a function that executes a command (that can be add at begin/at end, remove or play). When play command is executed, it should call the function print_list, but apparently it isn't being called, and the list is not being printed.
I tried to write the content of print_list in main function, checked if PLAY is getting its value of 2 so that execute_cmd calls correctly print_lists. Still, it doesn't print the content of the linked list.
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef enum commands { ADDBEG, ADDEND, PLAY, REMOVE, INVALID } command_t;
typedef struct song_s {
char name[99];
char time[20];
struct song_s *next;
} song_t;
void insert_end(song_t **p, song_t *entry);
void insert_begin(song_t **p, song_t *entry);
void remove_song(song_t **p, song_t *entry);
void execute_cmd(command_t command, song_t **head, song_t *entry);
bool already_present(song_t **p, song_t *new_song);
void print_list(song_t *p);
command_t split_input(char *input, song_t *song) {
command_t command = INVALID;
int len = strlen(input);
// Remove the newline character, if present
if (input[len - 1] == '\n') {
input[len - 1] = '\0';
}
// Check if the input command is "PLAY"
if (strcmp(input, "PLAY") == 0) {
return PLAY;
}
char *token = strtok(input, ",");
char *data[3];
int i = 0;
// Check if the input command starts with "REMOVE,"
if (input[0] == 'R') {
while (token != NULL && i < 2) {
data[i] = token;
token = strtok(NULL, ",");
printf("%s", data[i]);
i++;
}
return REMOVE;
}
// Check if the input command starts with "ADDBEG" or "ADDEND"
if (input[0] == 'A') {
while (token != NULL && i < 3) {
data[i] = token;
printf("%s\n", token);
token = strtok(NULL, ",");
i++;
}
}
// Set the command based on the input data
if (strcmp(data[0], "ADDBEG") == 0) {
command = ADDBEG;
} else if (strcmp(data[0], "ADDEND") == 0) {
command = ADDEND;
}
// Copy song data if available
if (data[1] != NULL)
strcpy(song->name, data[1]);
if (data[2] != NULL)
strcpy(song->time, data[2]);
return command;
}
int main(void) {
char input[100];
song_t song;
song_t *head = NULL;
int total_time = 0;
command_t command;
while (1) {
printf("input: ");
fgets(input, 100, stdin);
command = split_input(input, &song);
if (command == PLAY) {
printf("total time: %d\n", total_time);
break;
}
// Total time calculation
if (command == ADDBEG || command == ADDEND) {
if (!already_present(&head, &song)){
total_time += atoi(song.time);
}
}
if (command == REMOVE) {
total_time -= atoi(song.time);
}
// Execution based on command
execute_cmd(command, &head, &song);
}
return 0;
}
void insert_end(song_t **p, song_t *entry) {
song_t *q = (song_t *)malloc(sizeof(song_t));
memcpy(q, entry, sizeof(song_t));
q->next = NULL;
song_t *aux;
if (*p == NULL)
*p = q;
else {
aux = *p;
while (aux->next != NULL)
aux = aux->next;
aux->next = q;
}
}
void insert_begin(song_t **p, song_t *entry) {
song_t *q = (song_t *)malloc(sizeof(song_t));
memcpy(q, entry, sizeof(song_t));
q->next = *p;
*p = q;
}
void remove_song(song_t **p, song_t *entry) {
song_t *q = *p;
if (q == NULL)
return; // empty list
if (strcmp(q->name, entry->name) == 0) {
*p = q->next;
free(q);
return;
}
while (q->next != NULL) {
if (strcmp(q->next->name, entry->name) == 0)
break;
q = q->next;
}
if (q->next == NULL)
return; // value not found
song_t *tmp = q->next;
q->next = tmp->next;
free(tmp);
}
void print_list(song_t *p) {
while (p != NULL) {
printf("Song Name: %s, Time: %s\n", p->name, p->time);
p = p->next;
}
}
void execute_cmd(command_t command, song_t **head, song_t *new_song) {
// Call the appropriate function based on the command
if (command == ADDBEG) {
insert_begin(head, new_song);
} else if (command == ADDEND) {
insert_end(head, new_song);
} else if (command == REMOVE) {
remove_song(head, new_song);
} else if (command == PLAY) {
print_list(*head);
} else {
printf("Invalid command\n");
}
}
// Check if the song is already in the playlist
bool already_present(song_t **p, song_t *new_song) {
song_t *q = *p;
while (q != NULL) {
if (strcmp(q->name, new_song->name) == 0) {
return true;
}
q = q->next;
}
return false;
}
main(): break will terminate the while-loop so execute_cmd() never runs for the command PLAY.
split_input() segfault with input = "10" on strcmp() as both data and song are uninitialized and if either data, or the members name or time are missing a '\0' they are not strings and the calls to strcmp() and strcpy() would be undefined behavior.
split_input() with input[0] == 'A' and the rest of the line doesn't contain 2 commas, or if input[0] == 'R' and the rest of the line doesn't contain a comma. It's undefined behavior to call printf("%s\n", token) when token is NULL.
(Not fixed) Consider changing the char time[20] to an int or even better unsigned. Then do the text to number conversion in split_input().
(Not fixed) It's an interface design smell that you pass in song but it's only used for some of the commands.
(Not fixed) Consider renaming split_input() as it doesn't return array of strings that you would expect of a split function may or may not initialize the song_t output argument.
(Not fixed) It's odd that you process some of the action for command in main() and some in the respective functions. Also, as main() does similar command dispatch as execute_cmd() consdier merging the latter into main().
Prefer main() at the bottom of your file. That way you can often avoid declaring functions. This might be the first I have seen main() in the middle :-)
(Not fixed) Identifiers with a with a suffix _t are reserved.
If your song contains any comma then split_input() will assign part of the name to the time.
(Not fixed) split_input() doesn't process the time argument for REMOVE but main() expects it. I suggest change already_present() into song_t *find(song_t *p, const char *name) and use that to look up the time.
(Not fixed) The command REMOVE should only decrement total_time if the song is present.
(Not fixed) Prefer strtol() to atoi() as the latter has no way of reporting errors and default to 0.
Prefer using sizeof or a symbolic constant instead of repeating a magic value (100).
(Not fixed) As you only use !already_present() consider implementing the negation, say, is_new() or even better eliminate it in favor of find() (see above).
strcpy() is subject to buffer overflow if src is unconstrained.
(Not fixed) fgets() returns NULL on error (like EOF) you should check otherwise you end up with a busy loop (for instance when closing the stdin with ctrl-d).
(Not fixed) Consider making the head argument to print() a const i.e. void print_list(const song_t *p);`.
Prefer for to while-loops for iteration.
(Partially fixed) Prefer sizeof variable to sizeof(type).
Here's your revised program:
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define COMMANDS \
X(INVALID)\
X(ADDBEG)\
X(ADDEND)\
X(PLAY)\
X(REMOVE)
#define X(SYMBOL) SYMBOL,
typedef enum commands { COMMANDS } command_t;
#undef X
#define SEP ','
typedef struct song_s {
char name[99];
char time[20];
struct song_s *next;
} song_t;
void insert_end(song_t **p, song_t *entry);
void insert_begin(song_t **p, song_t *entry);
void remove_song(song_t **p, song_t *entry);
void execute_cmd(command_t command, song_t **head, song_t *entry);
bool already_present(song_t **p, song_t *new_song);
void print_list(song_t *p);
command_t split_input(char *input, song_t *song) {
input[strcspn(input, "\n")] = '\0';
char *s = strchr(input, SEP);
if(s) *s++ = '\0';
#define X(SYMBOL) {#SYMBOL, SYMBOL},
struct {
char *str;
command_t command;
} map[] = {
COMMANDS
};
#undef X
command_t command = INVALID;
for(size_t i = 0; i < sizeof map / sizeof *map; i++)
if(!strcmp(input, map[i].str)) {
command = map[i].command;
break;
}
if(command == ADDBEG || command == ADDEND) {
char *s2 = strrchr(s, SEP);
if(s2) {
*s2++ = '\0';
memset(song->name, 0, sizeof (song_t) {0}.name);
strncpy(song->name, s, sizeof (song_t) {0}.name - 1);
memset(song->time, 0, sizeof (song_t) {0}.time);
strncpy(song->time, s2, sizeof (song_t) {0}.time - 1);
} else
command = INVALID;
} else if(command == REMOVE) {
memset(song->name, 0, sizeof (song_t) {0}.name);
strncpy(song->name, s, sizeof (song_t) {0}.name - 1);
memset(song->time, 0, sizeof (song_t) {0}.time); // remove when `main()` uses find() to lookup the time.
}
return command;
}
void insert_end(song_t **p, song_t *entry) {
song_t *q = malloc(sizeof *q);
memcpy(q, entry, sizeof *entry);
q->next = NULL;
song_t *aux;
if (*p)
aux = *p;
while (aux->next != NULL)
aux = aux->next;
aux->next = q;
} else
*p = q;
}
void insert_begin(song_t **p, song_t *entry) {
song_t *q = malloc(sizeof *q);
memcpy(q, entry, sizeof *q);
q->next = *p;
*p = q;
}
void remove_song(song_t **p, song_t *entry) {
song_t *q = *p;
if (!q)
return; // empty list
if (!strcmp(q->name, entry->name)) {
*p = q->next;
free(q);
return;
}
for(; q->next; q = q->next)
if (strcmp(q->next->name, entry->name) == 0)
break;
if (!q->next)
return; // value not found
song_t *tmp = q->next;
q->next = tmp->next;
free(tmp);
}
void print_list(song_t *p) {
for(; p; p = p->next)
printf("Song Name: %s, Time: %s\n", p->name, p->time);
}
void execute_cmd(command_t command, song_t **head, song_t *new_song) {
if (command == ADDBEG)
insert_begin(head, new_song);
else if (command == ADDEND)
insert_end(head, new_song);
else if (command == REMOVE)
remove_song(head, new_song);
else if (command == PLAY)
print_list(*head);
else
printf("Invalid command\n");
}
// Check if the song is already in the playlist
bool already_present(song_t **p, song_t *new_song) {
for(song_t *q = *p; q; q = q->next)
if (!strcmp(q->name, new_song->name))
return true;
return false;
}
int main(void) {
char input[100];
song_t *head = NULL;
int total_time = 0;
for(;;) {
printf("input: ");
fgets(input, sizeof input, stdin);
song_t song;
command_t command = split_input(input, &song);
if (command == PLAY)
printf("total time: %d\n", total_time);
else if ((command == ADDBEG || command == ADDEND) && !already_present(&head, &song))
total_time += atoi(song.time);
else if (command == REMOVE)
total_time -= atoi(song.time);
execute_cmd(command, &head, &song);
}
}
and example session:
input: ADDBEG,Girls, Girls, Girls,3
input: PLAY
total time: 3
Song Name: Girls, Girls, Girls, Time: 3
input:
ctrl-c will terminate the program or you could add a quit command.