1

I've looked everywhere for an answer to my question, but I have yet to find a solid answer to my problem.

I'm currently in the process of writing a program in C, specifically targeting the UNIX command line (I'm using Linux as my development environment, but I'd like for this program to be as portable as possible). Right now, I have a basic shell that prompts for user input. The user will then enter in a command, and that command will be processed accordingly. Here is the code that I have so far:

/* Main.c */
int main(int argc, char **argv)
{
    while (TRUE)
    {
        display_prompt();
        get_command();
    }

    return 0;
}

/* Main.h */
void get_command()
{
    /*
     * Reads in a command from the user, outputting the correct response
     */

    int buffer_size = 20;   
    char *command = (char*) malloc(sizeof(char) * buffer_size);

    if (command == NULL)
    {
       return_error("Error allocating memory");
    }

    fgets(command, buffer_size, stdin);
    if (command[strlen(command) - 1] == '\n')
    {
        puts("It's inside the buffer.");
    }
    else
    {
        puts("It's not inside the buffer.");
    }

    free(command);
}

My initial thought was to check for the \n character and see if it fit within the buffer_size, and if it didn't realloc() the data to expand the allocated memory.

However, after I realloc() my string, how would I go about adding the remaining data from stdin into command?

Z. Charles Dziura
  • 933
  • 4
  • 18
  • 41

4 Answers4

5

Use getline(3) if you really need. It's POSIX.1-2008. And be aware that unlimited length lines are an easy attack vector for DOS-attacks (OOM). So consider making up a reasonable line length limit, and using fgets(3).

Jo So
  • 25,005
  • 6
  • 42
  • 59
2

You dont need do any realloc, just add 1 byte more than the maximum command length for \0 and forget \n because you wont get a \n always the user type enter. If the user input exceed the length then your string will be truncated without the \n. So your condition after fgets is not correct and based in a wrong assumption.

something like:

   int buffer_size = MAX_COMMAND_LENGTH + 1;

About memory allocation: you should use stack instead heap avoiding malloc/free in this case. So your code will be simpler and less error prone:

    char command[buffer_size];
    ...
    // free(command) <-- you dont need this anymore

Note that your command will be freed after function return. So if you will process it into get_command its ok but if you want return it to caller you shall receive a buffer from caller.

olivecoder
  • 2,858
  • 23
  • 22
  • Is COMMAND_MAX_LENGTH a built-in macro? Or will I have to define that myself? Also, for the condition checking in my original code, I know that the newline character may not exist. I used that as a check because the user will need to hit ENTER every time they want to send a command. If the newline character didn't exist, I wanted to expand the string to allow for more characters to be added. I simply didn't know how to do that. – Z. Charles Dziura Aug 11 '12 at 20:35
  • You need define the max length yourself or put a literal if you prefer. I was just trying be as clear as possible. – olivecoder Aug 11 '12 at 20:41
  • It is a user defined macro probably, at least I never heard of it as a built in. It makes more sense to be user defined anyway. – xQuare Aug 11 '12 at 20:42
  • I thought so, but I'm still really new to C, so I didn't know if I overlooked something simple like this. Also, to comment on your last edit, I don't want it to live in the stack because (eventually) I'll be returning the captured string back to the main loop. It's returning void right now for testing purposes. – Z. Charles Dziura Aug 11 '12 at 20:47
  • Of course you can make assumptions about the maximum command length because you know, or should know, what commands we are expecting. If you are using linux or some unix try in bash to get the bash maximum command length: getconf ARG_MAX – olivecoder Aug 11 '12 at 20:47
2

I think the answer about assuming a maximum command length is the correct one: typically you'll want to keep commands within a reasonable length.

But if you really can't make assumptions about the maximum length of a command, then you'll need to buffer.

Keep:

  • a fixed buffer that you always fgets the same number of characters into, and
  • a command that you append to, and realloc when necessary.

The following code probably lacks some error handling:

#define BUFFER_SIZE 20
#define COMMAND_BLOCK_SIZE 50

void get_command()
{
    char *buffer = malloc(sizeof(char) * (BUFFER_SIZE + 1));

    char *command = malloc(sizeof(char) * (COMMAND_BLOCK_SIZE + 1));
    int commandSize = 50;

    // tmp pointer for realloc:
    char *tmp = NULL;
    char *retval = NULL;

    if ((buffer == NULL) || (command == NULL))
        return_error("Error allocating memory");

    retval = fgets(buffer, BUFFER_SIZE, stdin);

    while (retval != NULL)
    {
        if (strlen(buffer) + strlen(command) > commandSize)
        {
            tmp = realloc(command, commandSize + (COMMAND_BLOCK_SIZE + 1));
            if (tmp == NULL)
                return_error("Error allocating memory");
            else
            {
                 command = tmp;
                 commandSize += COMMAND_BLOCK_SIZE;
            }
        }

        // not using strncat because the check above should guarantee that
        //    we always have more than BUFFER_SIZE more bytes in command 
        strcat(command, buffer);

        if (buffer[strlen(buffer) - 1] == '\n')
            break;

        retval = fgets(buffer, BUFFER_SIZE, stdin);
    }

    printf("COMMAND: %s\n", command);
    free(buffer);
}

Also note that:

  • we don't do anything useful with command there, you'll probably want to pass in a char ** so that you can get command out of this function, and free it in the calling code, e.g. in your main loop.
  • that the '\n' is retained in command: you may want to discard that.
pb2q
  • 58,613
  • 19
  • 146
  • 147
2

If you are using a gnu system, use the gnu getline extension to the c library, it does all the dynamic sizing for you.

e.g. (though I haven't tested)

void get_command()
{
    /*
     * Reads in a command from the user, outputting the correct response
     */

    size_t buffer_size = 0;   
    char *command = NULL;

    ssize_t len = getline(&command, &buffer_size, stdin);

    if(len < 0)
    {
        perror("Error reading input");
    }
    else if (command[len - 1] == '\n')
    {
        puts("It's inside the buffer.");
    }
    else
    {
        puts("It's not inside the buffer.");
    }

    free(command);
}
Kevin
  • 53,822
  • 15
  • 101
  • 132