-1

I'm trying to create a linked list of courses with dynamic allocation. I keep getting errors. can someone tell me what is the problem?

One of the errors is:

Dereferencing NULL pointer 'coursesList'.

Should I allocate memory to each of the fields of the struct?

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>

struct Course
{
    int a;
    char courseNumber[5];
    char courseName[30];
    struct Course* next;
};

struct Course* updateCoursesList(char courseName[], char courseNumber[]);


struct Course* updateCoursesList(char courseName[], char courseNumber[])
{
    struct Course* coursesList = (struct Course*)malloc(sizeof(struct Course));
    strcpy(coursesList->courseName, courseName);
    strcpy(coursesList->courseNumber, courseNumber);
    coursesList->next = NULL;
    coursesList->a = 2;
    printf("%d", coursesList->a);
    printf("%s", coursesList->courseName);
    printf("%s", coursesList->courseNumber);
    return coursesList;
}

int main()
{
    char newcourse[] = "math";
    char newcoursenumber[] = "54321";
    struct Course* A = updateCoursesList(newcourse, newcoursenumber);

}
underscore_d
  • 6,309
  • 3
  • 38
  • 64
Netanel
  • 21
  • 4
  • 3
    You're breaching the size limits of `courseNumber`. Storying `"54321"` requires **six** slots; not five (the terminating nulchar takes the extra slot). Therefore your program invokes *undefined behavior*, and with that come the ghosts and ghouls. – WhozCraig Dec 23 '20 at 11:44
  • @WhozCraig, indeed undefined behavior because the memory layout could suggest that printing course number would print `54321math`. I don't see how `coursesList` could become null. – Paul Ogilvie Dec 23 '20 at 11:55
  • Thank you, that helped. I changed the size of course number to 6. but I have a differernt error. At this row: strcpy(coursesList->courseName, courseName); The error is : Dereferencing NULL pointer 'coursesList'." – Netanel Dec 23 '20 at 11:55
  • You must check if `malloc` succeeded, i.e. `coursesList` is not null. – Paul Ogilvie Dec 23 '20 at 11:57
  • Fixing the issue I mentioned, [I cannot reproduce your error](https://coliru.stacked-crooked.com/a/3e11f3c3523f92d1). – WhozCraig Dec 23 '20 at 12:29
  • I don't get any errors when compiling your program. – Matteo Pinna Dec 23 '20 at 12:33
  • In addition, you should remember to `free` any dynamically allocated memory - `free(A);` in this case. Running your code, I couldn't reproduce the `NULL` pointer error -- but you should always check the return value of `malloc` as @paul points out, e.g. `if (coursesList == NULL) { return NULL; }`. – costaparas Dec 23 '20 at 14:02
  • Also aside: use `strncpy` instead -- it is safer, as described in [this](https://stackoverflow.com/questions/1258550/why-should-you-use-strncpy-instead-of-strcpy) post. – costaparas Dec 23 '20 at 14:04
  • @costaparas The accepted answer at the [link](https://stackoverflow.com/questions/1258550/why-should-you-use-strncpy-instead-of-strcpy) you suggested has a 20% down-vote. The higher rated [answer](https://stackoverflow.com/a/1258577/2410359) encourages a non-`strncpy()` solution. `strncpy()` is not really safer - it trades one set of problems for another. – chux - Reinstate Monica Dec 23 '20 at 18:54
  • @Netanel Please post a [mcve]. – chux - Reinstate Monica Dec 23 '20 at 19:02
  • @chux that's very true, thanks for pointing this out. Though `strncpy` isn't perfect is still has merit over `strcpy` since it *forces* the programmer to *think* about the size of the buffers; oftentimes that's enough to catch issues early on. But using `strcat` to do a copy is more cryptic than it should be. Perhaps `snprintf` would be better overall. – costaparas Dec 23 '20 at 23:33
  • @costaparas I'd consider using `char *courseName;` and later `coursesList->courseName = strdup(courseName);` – chux - Reinstate Monica Dec 23 '20 at 23:36
  • @chux that's also suitable, assuming the struct definition is allowed to change here, and the memory is freed afterwards. – costaparas Dec 23 '20 at 23:46

1 Answers1

0
  1. input to the function is a char array, not a string, strcpy will happen till it find a a "\0".

  2. Please use strncpy with array size to avoid crash, and overwriting of data or corrupting other fields in structure.

  3. courseNumber size if not enough to hold 5 char string plus "\0" ,\0" is required for string opeartions otherwise should print char's in array. keeping value 5 will give output

     2
     math
     54321math
    
  4. used "size()-1" intentionally to add "\0" in stncpy , incase if input char array donot have a "\0" will be placed one by memset and stncpy.

     #include <stdio.h>
     #include <stdlib.h>
     #include <ctype.h>
     #include <string.h>
    
     struct Course
     {
         int a;
         char courseNumber[6];
         char courseName[30];
         struct Course* next;
     };
    
     struct Course* updateCoursesList(char courseName[], char courseNumber[]);
    
     struct Course* updateCoursesList(char courseName[], char courseNumber[])
     {
         struct Course* coursesList = (struct Course*)malloc(sizeof(struct Course));
         if (coursesList == NULL) {
             return coursesList;
         }
         memset(coursesList, 0, sizeof(struct Course));
    
         strncpy(coursesList->courseName, courseName,    sizeof(coursesList->courseName)-1);
         strncpy(coursesList->courseNumber,courseNumber, sizeof(coursesList->courseNumber)-1);
         coursesList->next = NULL;
         coursesList->a = 2;
         printf("%d\n", coursesList->a);
         printf("%s\n", coursesList->courseName);
         printf("%s\n", coursesList->courseNumber);
         return coursesList;
     }
    
    
     int main()
     {
         char newcourse[] = "math\0";
         char newcoursenumber[] = "54321\0";
         struct Course* A = updateCoursesList(newcourse, newcoursenumber);
         // do operations here
    
         // free before exit
         if (A != NULL) {
             free(A);
             // good practice to set vale to null after free
             A = NULL;
         }
         return 0;
     }
    
Nishad C M
  • 142
  • 6
  • `strncpy()` is not enough to insure `printf("%s\n", coursesList->courseName);` works. – chux - Reinstate Monica Dec 23 '20 at 15:44
  • size is kept as "sizeof(coursesList->courseNumber)-1" for strncpy, array is set to "0" and this will write one char less than array size, make sure there is a termination char. – Nishad C M Dec 23 '20 at 15:50
  • True. (the -1, far to the right) was not visible in my screen w/o scrolling, yet it does maintain the null character termination as you say. See also [Why is strncpy insecure?](https://stackoverflow.com/a/10845959/2410359), – chux - Reinstate Monica Dec 23 '20 at 16:46
  • Please remember to free all allocated memory when posting "corrected" code. Also, you [don't](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) need to cast the return value of malloc. And also, its more correct to compare the return value with the `NULL` macro rather than `0`. – costaparas Dec 23 '20 at 23:50