-1

I am trying to allocate memory to a 2d character array as I determine its size. (the count is assumed to be an unknown value) it seems to work until something starts to reassign garbage data to the array

0xd28fe280 -> 3
0xd28fe280 -> 3
0xd28fe280 -> 3
0xd28fe280 -> 3
0xd28fe280 -> ���[U
0xd28fe280 -> ���[U

what essentially what I want to do is allocate memory just before I populate the array with strings.

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

int main(){
  int count = 6;
  char **words;
  words = malloc(0);
  for(int i = 0;i < count; i++){
    words[i] = malloc(10);
    strcpy(words[i],"3");
    printf("%#08x -> %s\n",words[0],words[0]);
  }
  free(words);
  return 0;
}
  • 2
    I'd say its time to learn how to use a debugger. – Swordfish Nov 07 '18 at 20:11
  • Why 10 in `malloc(sizeof(char) * 10)`? – chux - Reinstate Monica Nov 07 '18 at 20:12
  • This doesn't make sense: `printf("%2i << %#08x adress has %s in it\n", j, uniquNodes[j], uniquNodes[j]);` – Fiddling Bits Nov 07 '18 at 20:12
  • 8
    `uniquNodes = malloc(0);` Ahm ... – Osiris Nov 07 '18 at 20:13
  • 1
    Can you explain what your code is supposed to do, and how it's supposed to do it? Compare what you've posted with the idea of a [mcve] – Tim Randall Nov 07 '18 at 20:14
  • 3
    `uniquNodes = malloc(0); ... uniquNodes[count] = ...` is UB as `uniquNodes` does not point to sufficient memory. – chux - Reinstate Monica Nov 07 '18 at 20:15
  • `if (uniquNodes == NULL) { free(uniquNodes); }`.. – Tormund Giantsbane Nov 07 '18 at 20:16
  • Not sure what you're doing, but I can't remember the last time I used four nested `for` loops. Make sure you understand your algorithm before coding,, and I'd recommend splitting that up into multiple functions... there's a whole lot going on in `relationTest` – yano Nov 07 '18 at 20:18
  • 2
    @TormundGiantsbane While it is nonsense it is actually still legal. – Osiris Nov 07 '18 at 20:23
  • 2
    `char pairs[numPairs][2][10];` seems like a recipe for disaster. Have you considered using an *array of struct* containing the pairs? – David C. Rankin Nov 07 '18 at 20:25
  • Subclause 7.22.3 C Standard [ISO/IEC 9899:2011]: "If the size of the space requested is zero, the behavior is implementation-defined: either a null pointer is returned, or the behavior is as if the size were some nonzero value, except that the returned pointer shall not be used to access an object." Have you tried `realloc`? – Neil Nov 07 '18 at 21:46
  • @chux because I want 10 to be the max size of the character array, I could probably dynamically allocate just enough by getting the length of the string, but I want to get this working first. – SpeedrunnerG55 Nov 07 '18 at 23:06
  • @SpeedrunnerG55 Do you see now that `uniquNodes = malloc(0);` is insufficient memory allocation? – chux - Reinstate Monica Nov 07 '18 at 23:11
  • @FiddlingBits that line is supposed to print the address of the character array and the string it contains, so I can make sure that my pointer didn't change and i can see if the value changed. normally I would not have that in the function it's just me trying to see why the string became what it is – SpeedrunnerG55 Nov 07 '18 at 23:28
  • @chux yes, but every time before i try to assign it any data i do `uniquNodes[count] = malloc(sizeof(char) * 10);` to allocate it 10 more bytes for each new string i add, it starts out as 0, and should grow as needed. unless im missing something – SpeedrunnerG55 Nov 07 '18 at 23:31
  • @NeilEdelman ive tried `if(realloc(uniquNodes,(count+1) * sizeof(char) * 10) == NULL){ free(uniquNodes); printf("REALLOC FAILLED"); exit(0); }` the error message still says "REALLOC" from when i tried to use this but it wont even run – SpeedrunnerG55 Nov 07 '18 at 23:37
  • @TimRandall ill cut down the part of the function down and give it example data so it can run by itself – SpeedrunnerG55 Nov 07 '18 at 23:40
  • It's very easy to slip up in memory allocation with your current system. Encourage you to follow @DavidC.Rankin's advice; use `struct`s that enforce your meaning. See https://stackoverflow.com/questions/21006707/proper-usage-of-realloc. – Neil Nov 08 '18 at 00:08
  • @NeilEdelman using `struct Pair{ char p[2][10]; };` seems good idea but declaring each value manually for the shortened example is a lot more tedious than just using the 3d array, ill keep it in mind once i get the rest working tho – SpeedrunnerG55 Nov 08 '18 at 03:45
  • The edit changed `uniquNodes = malloc(0);` to `words = malloc(0);` so the first comment made by @Osiris, above, that shows a big error still applies. – AdrianHHH Nov 08 '18 at 09:21

1 Answers1

1

It is actually not a 2D array, it is a pointer to a character pointer (char **).

words point to a block of char *, where each element of this block points to a char block. You only have allocated memory for the char blocks, but not for the char * block. (You have allocated it with size 0 so you can not access it). You also need to free every block you allocated, otherwise the memory is leaked. It would be also good practice to check the return value of malloc, since it returns NULL if it fails and further dereferencing a NULL pointer will lead to undefined behavior.

This should work:

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

int main()
{
    int count = 6, max_len = 10, words_n = 0;
    char **words = NULL;

    for(int i=0; i<count; i++)
    {
        words = realloc(words, ++words_n * sizeof *words);
        if(!words)
        {
            //Error handling
            return -1;
        }
        words[i] = malloc(max_len * sizeof *words[i]); 
        if(!words[i])
        {
            //Error handling
            return -1;
        }
        strncpy(words[i], "3", max_len); //Better to protect against overflows.
        words[i][max_len-1] = '\0';
        printf("%p -> %s\n", (void*)words[0], words[0]); //"%p" for printing pointers.
    }

    for(int i=0; i<count; i++)
    {
        free(words[i]); //Free every allocated element.
    }
    free(words);

    return 0;
}
Osiris
  • 2,783
  • 9
  • 17
  • @SpeedrunnerG55 Its similar to that, but i have adjusted my answer. Note that the return value of `realloc` needs to be assigned to the pointer since the place can change. – Osiris Nov 08 '18 at 16:40
  • since i believe `*words[i]` is a char would `sizeof *words[i]` always be 1, could just do `words[i] = malloc(max_len)`? – SpeedrunnerG55 Nov 08 '18 at 17:57
  • 1
    @SpeedrunnerG55 Yes i only wrote it like that to be consistent. – Osiris Nov 08 '18 at 18:07