-1

char **res = (char **)malloc(sizeof(char *) * 1) at this line i have used {sizeof(char *) * 1} but i have placed more than one string with different length. I dont get how is this working, or is it just my compiler not showing error/warning or is this correct.

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

int main() {
    char **res = (char **)malloc(sizeof(char *) * 1);
    res[0] = "mang0000000o";
    res[1] = "tango00000";
    res[2] = "lango";
    res[3] = "django";
    for (int x = 0; x < 4; x++) {
        puts(res[x]);
        putchar('\n');
    }
    return 0;
}
Anonymous
  • 318
  • 4
  • 14
  • 4
    Your compiler isn't required to issue a diagnostic (error or warning) when your code has undefined behaviour, as it does here. It's entirely possible your program will appear to work, at least for a while. See [here](https://stackoverflow.com/questions/2397984/undefined-unspecified-and-implementation-defined-behavior) for more information and further links. – Useless Dec 14 '21 at 15:35
  • 3
    syntactically this is correct, but you have not allocated enough space for the strings you assign, so you're invoking undefined behavior. Change that to `sizeof(char *) * 4`. – yano Dec 14 '21 at 15:36
  • 1
    GCC warns you: https://godbolt.org/z/4vjMMx48G – mch Dec 14 '21 at 15:39
  • 3
    It "works" in the same way that you can cram 10 people into a vehicle made for 5, but they'll spill their lunch on each other. – Weather Vane Dec 14 '21 at 15:42
  • @mch that was awesome. – Anonymous Dec 14 '21 at 15:43
  • Compile with `-Wall -Wextra -fsanitize=address -O2` – klutt Dec 14 '21 at 15:44
  • @klutt and at least `-O2`. Without optimization it does not find it. – mch Dec 14 '21 at 15:44
  • 2
    [`-g -fsanitize=address,undefined`](https://godbolt.org/z/cs9er4js7) gives good runtime info. It points directly at `res[1] = "tango00000";` "_AddressSanitizer: heap-buffer-overflow_" – Ted Lyngmo Dec 14 '21 at 15:49

2 Answers2

0

In this case, you have allocated memory for an array of pointers of length 1. And when you turned to an element that you didn't allocate memory for, you just turned to the next piece of memory after the first element of the arrays. If you try to do this in a cycle of at least a hundred elements, you are almost guaranteed to get a segmentation fault. The error does not occur when you access memory that you have not allocated, but when you access memory already occupied by someone.

In such cases, you should allocate memory for sizeof(char *) * (n + 1) where n is the number of elements you need. You will write NULL to the last pointer, so that it can be conveniently iterated through while or for.

Gumada Yaroslav
  • 115
  • 1
  • 10
  • Good attempt at an answer but has some issues: "cycle of 100 elements" ? It is probably a problem before then. It's undefined behavior at `res[1] =` (undefined behavior is the key phrase here). You can allocate n+1 or you can just allocate n and keep track of n. Also, the array is of char* but string literals are const so that could be an issue depending what you try to do. – John3136 Dec 14 '21 at 16:51
  • @John3136 This is the problem of undefined behavior, so c programs need to be run many times and preferably on a large amount of data. Otherwise, you can't guarantee that you won't get a floating error. That is why many other languages have abandoned memory management, and even in C++ almost no one allocates memory directly through malloc – Gumada Yaroslav Dec 14 '21 at 16:58
  • I'm aware of all that but trying to put a number of what will cause undefined behavior is not possible. The code I'm porting now crashes if you go one off the end of an array. No one should be using malloc ever in C++ - they should be using new. – John3136 Dec 14 '21 at 22:18
-1

This should work:

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

 int main() {
     const char **res = malloc(4 * sizeof *res); // Buff the size up a little bit
     res[0] = "mang0000000o";
     res[1] = "tango00000";
     res[2] = "lango";
     res[3] = "django";

     for (int x = 0; x < 4; x++) {
        puts(res[x]);
        putchar('\n'); // Note that 'puts' automatically adds a newline character '\n'
    }
    
    free(res);

    return 0;
}

Also note that you don't necessarily need the string.h header. Hope this worked out for you.

TechTycho
  • 134
  • 1
  • 10
  • Your `* 15` here is completely unneeded. – SergeyA Dec 14 '21 at 16:21
  • I know, this is just to make it easier, determining the max size of an element. You could just remove it and buff up that '4' to something like 30 or something. – TechTycho Dec 14 '21 at 16:24
  • Why do you you want to multiply it by anything at all? – SergeyA Dec 14 '21 at 16:30
  • The pointer type can be deceiving. I suggest `const char **res = malloc(4 * sizeof *res);` instead - and also, `free(res);` at the end. – Ted Lyngmo Dec 14 '21 at 16:30
  • @SergeyA Please test it without multiplying and see what you get. I just multiply because the `sizeof(char)` is just one byte; do you think all of this is one byte?! – TechTycho Dec 14 '21 at 16:36
  • Thanks @TedLyngmo I'll make it better. – TechTycho Dec 14 '21 at 16:36
  • 2
    Why would you want to take `sizeof char` if you store elements of type `char*`? Your buffer is too small to hold 4 pointers on a 64bit system. The original code was actually closer to the solution than that. only the `*1` should be changed to `*4`. – Gerhardh Dec 14 '21 at 16:38
  • 1
    You want to `malloc` space for the type of object the pointer will point to. Since `res` is a `char**`, you should `malloc` space for `char*` types. That is, `char*` type in this case is what you should feed to `sizeof`, times however many objects you want. `sizeof(char*)` or better yet `sizeof(*res)` as Ted mentions, since `*res` will always give you the correct object type no matter what kind of pointer `res` is. – yano Dec 14 '21 at 16:40
  • Doesn't actually answer the question. – John3136 Dec 14 '21 at 16:40
  • Oh thanks, didn't know that! Please don't get mad at me.. I fixed it.. – TechTycho Dec 14 '21 at 16:41
  • 1
    @TechTycho I think you missed the point I made about that the type can be deceiving. The strings are filled with `const char`s so making `char*`'s to them can lead to problems later on (someone may try to change the strings - and it'll compile just fine but lead to UB). You also do not need to cast the return from `malloc` in C, so `const char **res = malloc(4 * sizeof *res);` would be fine. – Ted Lyngmo Dec 14 '21 at 16:44