1

I'm working on a function, that has to take a dynamic char array, separate it at spaces, and put each word in an array of char arrays. Here's the code:

char** parse_cmdline(const char *cmdline)
{
    char** arguments = (char**)malloc(sizeof(char));
    char* buffer;
    int lineCount = 0, strCount = 0, argCount = 0;
    int spaceBegin = 0;
    while((cmdline[lineCount] != '\n'))
    {
        if(cmdline[lineCount] == ' ')
        {
            argCount++;
            arguments[argCount] = (char*)malloc(sizeof(char));
            strCount = 0;
        }
        else
        {
            buffer = realloc(arguments[argCount], strCount + 1);
            arguments[argCount] = buffer;
            arguments[argCount][strCount] = cmdline[lineCount];
            strCount++;
        }
        lineCount++;
    }
    arguments[argCount] = '\0';
    free(buffer);
    return arguments;   
}

The problem is that somewhere along the way I get a Segmentation fault and I don't exacly know where. Also, this current version of the function assumes that the string does not begin with a space, that is for the next version, i can handle that, but i can't find the reason for the seg. fault

Improve this question
6
  • 3
    char * and char ** are not an arrays! A pointer is not an array! Commented Jan 3, 2016 at 19:01
  • 3
    Use a debugger to find out where. And do not cast malloc() to the target pointer type. Read the frequents tab for c, the first question. Commented Jan 3, 2016 at 19:01
  • 2
    Also, malloc(sizeof(char)) will only allocate a single byte. Commented Jan 3, 2016 at 19:07
  • @Olaf Although a pointer isn't an array, it can be used as an array if it is malloced with enough space. (The pointer arithmetic can be done using brackets which is what I think the person is trying to do, i "believe"). Commented Jan 3, 2016 at 19:22
  • 1
    @JonathanSmit: You refer to the subscription operator, I assume. That is just because an array decays to a pointer for most cases, including this operator. And even when it points to a sequence of variables of the same type, a pointer is not an array. Try sizeof(char *) or char * = &(char [4])` (type-expressions). And a char ** is still not an "array of arrays". Also, the title is very missleading. It implies to convert/cast types, while the question actually is about processing data from one array and store the results into another. Commented Jan 3, 2016 at 20:34

4 Answers 4

Reset to default
1

This code is surely not what you intended:

char** arguments = (char**)malloc(sizeof(char));

It allocates a block of memory large enough for one char, and sets a variable of type char ** (arguments) to point to it. But even if you wanted only enough space in arguments for a single char *, what you have allocated is not enough (not on any C system you're likely to meet, anyway). It is certainly not long enough for multiple pointers.

Supposing that pointers are indeed wider than single chars on your C system, your program invokes undefined behavior as soon as it dereferences arguments. A segmentation fault is one of the more likely results.

The simplest way forward is probably to scan the input string twice: once to count the number of individual arguments there are, so that you can allocate enough space for the pointers, and again to create the individual argument strings and record pointers to them in your array.

Note, too, that the return value does not carry any accessible information about how much space was allocated, or, therefore, how many argument strings you extracted. The usual approach to this kind of problem is to allocate space for one additional pointer, and to set that last pointer to NULL as a sentinel. This is much akin to, but not the same as, using a null char to mark the end of a C string.

Edited to add:

The allocation you want for arguments is something more like this:

arguments = malloc(sizeof(*arguments) * (argument_count + 1));

That is, allocate space for one more object than there are arguments, with each object the size of the type of thing that arguments is intended to point at. The value of arguments is not accessed by sizeof, so it doesn't matter that it is indeterminate at that point.

Edited to add:

The free() call at the end is also problematic:

free(buffer);

At that point, variable buffer points to the same allocated block as the last element of arguments points to (or is intended to point to). If you free it then all pointers to that memory are invalidated, including the one you are about to return to the caller. You don't need to free buffer at that point any more than you needed to free it after any of the other allocations.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

This is probably why you have a segmentation fault:

In char** arguments = (char**)malloc(sizeof(char));, you have used malloc (sizeof (char)), this allocates space for only a single byte (enough space for one char). This is not enough to hold a single char* in arguments.

But even if it was in some system, so arguments[argCount] is only reading allocated memory for argCount = 0. For other values of argCount, the array index is out of bounds - leading to a segmentation fault.

For example, if your input string is something like this - "Hi. How are you doing?", then it has 4 ' ' characters before \n is reached, and the value of argCount will go up till 3.

Improve this answer

Comments

1

What you want to do is somthing like this:

char** parse_cmdline( const char *cmdline )
{

Allocate your array of argument pointers with length for 1 pointer and init it with 0.

char** arguments = malloc( sizeof(char*) );
arguments[0] = NULL;

Set a char* pointer to the first char in yor command line and remember the beginn of the first argument

int argCount = 0, len = 0;
const char *argStart = cmdline;
const char *actPos = argStart;

Continue until end of command line reached. If you find a blank you have a new argument which consist of th characters between argStart and actPos . Allocate and copy argument from command line.

while( *actPos != '\n' && *actPos != '\0' )
{
    if( cmdline[lineCount] == ' ' && actPos > argStart )
    {
        argCount++; // increment number of arguments
        arguments = realloc( arguments, (argCount+1) * sizeof(char*) ); // allocate argCount + 1 (NULL at end of list of arguments)
        arguments[argCount] = NULL; // list of arguments ends with NULL
        len = actPos - argStart;
        arguments[argCount-1] = malloc( len+1 ); // allocate number of characters + '\0'
        memcpy( arguments[argCount-1], actPos, len ); // copy characters of argument
        arguments[argCount-1] = 0; // set '\0' at end of argument string
        argStart = actPos + 1; // next argument starts after blank
    }
    actPos++;
}
return arguments;  

}
Improve this answer

Comments

0

some suggestions i would give is, before calling malloc, you might want to first count the number of words you have. then call malloc as char ** charArray = malloc(arguments*sizeof(char*));. This will be the space for the char ** charArray. Then each element in charArray should be malloced by the size of the word you are trying to store in that element. Then you may store that word inside that index. Ex. *charArray = malloc(sizeof(word)); Then you can store it as **charArray = word;

Be careful with pointer arithmetic however.

The segmentation fault is definitly arising from you trying to access an element in an array in an undefined space. Which arises from you not mallocing space correctly for the array.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.