Manipulation of arrays of strings in C

Question

I'm working on a basic shell (as in the console program that awaits commands and executes them in UNIX systems) replica in C, and need to be able to manipulate 2d arrays of char to store the environment variables.

I wrote a small function to create that 2d array and initialize each string to NULL before I fill it up elsewhere in my code. Except that it crashes as soon as the program is launched, for some reason.

I have similar issues (namely occasional segfaults, probably due to me reading/writing in an inapropriate place) with two other functions, respectively to free those 2d arrays when needed, and to get the length of one of those 2d array.

If I don't use these two functions and malloc the 2d array within the rest of my code, without initializing anything except the last entry to NULL, but instead copy the env strings directly after the malloc, I have something that works. But it'd be better to be able to prevent the memory leaks, and to have that ft_tabnew function to work so that I could reuse it in future projects.

char        **ft_tabnew(size_t size)
{
    char    **mem;
    size_t  i;

    if (!(mem = (char **)malloc(size + 1)))
        return (NULL);
    i = 0;
    while (i < size + 1)
    {
        mem[i] = NULL;
        i++;
    }
    return (mem);
}

void    ft_tabdel(char ***as)
{
    int i;
    int len;

    if (as == NULL)
        return ;
    i = 0;
    len = ft_tablen(*as);
    while (i < len)
    {
        if (*as[i])
            ft_strdel(&(*as[i]));
        i++;
    }
    free(*as);
    *as = NULL;
    return ;
}

size_t      ft_tablen(char **tab)
{
    size_t  i;

    i = 0;
    while (tab[i])
        i++;
    return (i);
}

NOTE : The ft_strdel function used in ft_tabdel is freeing a string that was dynamically allocated, and sets the pointer to NULL. I've been using it for a few months in several projects and it has not failed me yet.

Hopefully, you wonderful people will be able to tell me what misconception or misunderstanding I have about 2d arrays of chars, or what stupid error I'm making here.

Thank you.

[don't cast malloc](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Barmar, Jan 31 '15 at 16:34
Apart from Barmar's answer (with which I agree), (1) use `memset` to set entire ranges to 0, (2) use `for` loops instead of those `x=0;while(x<..) x++;`; they are made for it. — Jongware, Jan 31 '15 at 16:39
@Jongware, (1) I'll update my code acording to this advice. (2) I would, but I can't. This project is for school, and we are forced to respect certain contraints, some of them being no for loops, 25 lines max per functions. I presume this is to teach us to write concise code, and to not hesitate to break down functions to keep our code as short and readable as possible. Or maybe just to mess with our heads because of reasons =P — kRYOoX, Jan 31 '15 at 17:06
I think you you should probably be using a structure to describe those arrays, such as `struct ft_tab { char **array; size_t max_len; size_t act_len; };` where the `max_len` member records how much space is allocated and the `act_len` records how much of the space is in use. You can pass these structures to functions, and the `ft_tablen()` function becomes redundant or trivial (`static inline size_t ft_tablen(const struct ft_tab *ft) { return ft->act_len; }`). This allows you avoid passing [three-star pointers](http://c2.com/cgi/wiki?ThreeStarProgrammer) around, which is better for everyone. — Jonathan Leffler, Jan 31 '15 at 17:19

score 4 · Accepted Answer · answered Jan 31 '15 at 16:36

4

You're not allocating enough space.

if (!(mem = (char **)malloc(size + 1)))

only allocates size+1 bytes. But you need to allocate space for size+1 pointers, and pointers are typically 4 bytes. You need to multiply the number of elements by the size of each element:

    if (!(mem = malloc((size + 1) * sizeof(*mem))))

answered Jan 31 '15 at 16:36

Barmar

741,623
53
500
612

That indeed fixed the crash on launch. I knew I was making a stupid mistake ... Thank you sir. I'll let the topic run a little more for suggestions on the other issues. – kRYOoX Jan 31 '15 at 16:45

score 1 · Answer 2 · answered Jan 31 '15 at 16:40

In the code

char **mem;
while (i < size + 1)
{
    mem[i] = NULL;
    i++;
}

mem is a "pointer to a pointer to a char" and hence its size is that of a pointer, not of a char. When you say mem[i] and incement i, you increment with the size of pointer, not of char, and so overwrite memory outside your allocated memory. Try:

if (!(mem = (char **)malloc((size + 1)*sizeof(void *))))

Manipulation of arrays of strings in C

2 Answers2