0

I'm new to C, coming from another language and I'm getting this error that I understand now but couldn't find how to fix it inside of my code.

It's pretty simple but even when debugging with printf or perror I couldn't figure it out because it returns Success on the memory allocation.

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

char *tasks[6];

// Made the size 5 to allow for `\0`
tasks[0] = (char *) malloc(5 * sizeof(char));
tasks[1] = (char *) malloc(5 * sizeof(char));
tasks[2] = (char *) malloc(5 * sizeof(char));
tasks[3] = (char *) malloc(5 * sizeof(char));
tasks[4] = (char *) malloc(5 * sizeof(char));
tasks[5] = (char *) malloc(5 * sizeof(char));

for (int i = 0; i < sizeof(tasks); i++) {
    if (!tasks[i]) {
        perror("malloc result : "); // Returns Success
    }
}

strncpy(tasks[0], "Dish", 5);
strncpy(tasks[1], "Laun", 5);
strncpy(tasks[2], "Bath", 5);
strncpy(tasks[3], "Flor", 5);
strncpy(tasks[4], "Tras", 5);
strncpy(tasks[5], "Offi", 5);

for (int i = 0; i < sizeof(tasks); i++) {
    printf("%s\n", tasks[i]);
}

for (int i = 0; i < sizeof(tasks); i++) {
    free(tasks[i]);
}
Improve this question
15
  • 1
    This loop for (int i = 0; i < sizeof(tasks); i++) { invokes undefined behavior. Instead you should write for (size_t i = 0; i < sizeof(tasks) / sizeof( *tasks ); i++) { Commented Feb 7 at 9:40
  • 2
    sizeof is in bytes, not in number of items (which are pointers, so 4 or 8 bytes long) Commented Feb 7 at 9:41
  • 3
    This question is similar to: How do I determine the size of my array in C?. If you believe it’s different, please edit the question, make it clear how it’s different and/or how the answers on that question are not helpful for your problem. Commented Feb 7 at 9:41
  • 1
    This could have been easily avoided by creating some constant #define TASKS_SIZE 6 and then only ever use that one when referring to the size of the array. The root cause of the bug is "magic numbers" in combination of misunderstanding how sizeof works. Commented Feb 7 at 13:35
  • 1
    @Jabberwocky -- no, you are right: the C Standard doesn't require malloc to set errno; I was just pointing out that these common implementations of the Standard Library do set it. Commented Feb 7 at 18:43

1 Answer 1

Reset to default
3

Is because you don't take the good size of your array:

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


int main(void){

    char *tasks[6] = {NULL};
    
    // Made the size 5 to allow for `\0`
    tasks[0] = (char *) malloc(5 * sizeof(char));
    tasks[1] = (char *) malloc(5 * sizeof(char));
    tasks[2] = (char *) malloc(5 * sizeof(char));
    tasks[3] = (char *) malloc(5 * sizeof(char));
    tasks[4] = (char *) malloc(5 * sizeof(char));
    tasks[5] = (char *) malloc(5 * sizeof(char));
    
    for (int i = 0; i < sizeof(tasks) / sizeof(tasks[0]); i++) {
        if (!tasks[i]) {
            perror("malloc result : "); // Returns Success
        }
    }
    
    strncpy(tasks[0], "Dish", 5);
    strncpy(tasks[1], "Laun", 5);
    strncpy(tasks[2], "Bath", 5);
    strncpy(tasks[3], "Flor", 5);
    strncpy(tasks[4], "Tras", 5);
    strncpy(tasks[5], "Offi", 5);
    
    for (int i = 0; i < sizeof(tasks) / sizeof(tasks[0]); i++) {
        printf("%s\n", tasks[i]);
    }
    
    for (int i = 0; i < sizeof(tasks) / sizeof(tasks[0]); i++) {
        free(tasks[i]);
    }

}

Give the right result without any memory error:

==1026== Memcheck, a memory error detector
==1026== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1026== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==1026== Command: ./a.out
==1026==
Dish
Laun
Bath
Flor
Tras
Offi
==1026==
==1026== HEAP SUMMARY:
==1026==     in use at exit: 0 bytes in 0 blocks
==1026==   total heap usage: 7 allocs, 7 frees, 1,054 bytes allocated
==1026==
==1026== All heap blocks were freed -- no leaks are possible
==1026==
==1026== For lists of detected and suppressed errors, rerun with: -s
==1026== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

Also worth noting, in C, there is no need to cast the return of malloc, it is unnecessary. See: Do I cast the result of malloc? Also helpful to remind them to validate every allocation. It's just a matter of "when" not "if" an allocation failure may occur.
I suggest to do the call to malloc and the error handling in one place. If malloc would set errno, you would only get the last error and not the possibly different errors of multiple failed calls. And the error handling of malloc's result should not only print an error message but also prevent the subsequent code from using it. If any tasks[i] is NULL, you must not use it as an argument for strncpy or printf.
Guys ! I just fixed he's code... with the right size of the array... I don't review this code...
@Asile34, the comments are at least as much for the OP as they are for you. You don't necessarily have to modify your answer in light of them, and you should not take them as criticism. Be aware, however, that it is a common practice around here for answerers to provide additional, secondary information such as is in those comments.
I’m confused, why divide it with the same array at position [0] ? Couldn’t I just do i < tasks[0] then ?
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.