confused about get strings to a two dimensional array

Question

I'm trying to write a simple C program to read user input, and write it back to stdout. I used gets() to get lines of input from stdin (which is what my teacher asked me to do). I can read lines, but I cannot get out of the while loop. For example, if I input 10 lines, when I run the program I find that my code is reading the 11th line, which is empty, and does not exit the loop. This is my code:

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

#define MAX_LINE_SIZE 100
#define INITIAL_BUFFER_SIZE 16

char** IncreaseBuffer(char** buffer, int* buffer_size);
void PrintLines(char** lines, int total_lines);
int MinLineIndex(char** buffer, int i, int j);
void SwapLines(char** buffer, int i, int j);

int main(){
    char** buffer;
    int* buffer_size;
    int* line_size;
    int s,t;
    int i=0;
    char* q;
    s = INITIAL_BUFFER_SIZE;
    buffer_size = &s;
    t = MAX_LINE_SIZE;
    line_size = &t;
    printf("Get lines:" );
    buffer = (char**)malloc(sizeof(char*)**buffer_size);`enter code here`
    buffer[0] = malloc(sizeof(char)**line_size);
    while(1){
        q = gets(buffer[i]);
        *buffer[i] = *q;
        if(buffer[i] == NULL){
            i = i - 1;
            char null = '\0';
            buffer[i+1] = &null;
            *buffer[i+1] = '\0';
            break;
        }
        i++;
        buffer[i] = malloc(sizeof(char)**line_size);
        printf("%d%s\n",i,buffer[i]);
    }
}

What have I done wrong?

Answer 1

While you can read an unknown number of lines in several ways, your approach and insistence on declaring pointers is a bit cumbersome. There is no need to declare buffer_size or line_size or q as pointers. s and t can be given a bit more descriptive names to help with readability.

First, understand, you are not creating a two dimensional array below. You are using a pointer-to-pointer-to-char to declare an array of pointers to which you are assigning the address for an allocated block of memory holding each line of input. You can use array indexes to access each individual pointer value (e.g. buffer[0] for the first pointer, and so on).

Since you are declaring an INITIAL_BUFFER_SIZE number of pointers with buffer, if you rename s to ptrcount you can use ptrcount to keep track to the current number of pointers allocated, and realloc when the count is reached. With t, if you are not reallocating the number of characters per line, then there isn't a reason to declare a separate variable to track MAX_LINE_SIZE, you can simply use that as the constant.

It is often useful, especially in cases where you want to check the contents read from the user before allocating storage for the line, to use a static buffer for the initial read, say:

char buf[MAX_LINE_SIZE] = {0};

This allows for your check for an empty line, as well as your removal of the trialing '\n' before you allocate space in buffer[i] and copy the string to its final location. (note: strdup can be used to both allocate and copy to buffer[i])

Putting the pieces together, you could accomplish your filling (and freeing) of buffer -- along with its reallocation if you read more than INITIAL_BUFFER_SIZE lines similar to the following:

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

enum { INITIAL_BUFFER_SIZE = 16, MAX_LINE_SIZE = 100 };

int main (void) {

    char buf[MAX_LINE_SIZE] = {0};
    char **buffer;
    int ptrcount, nlines;
    int i = 0;

    ptrcount = INITIAL_BUFFER_SIZE;

    printf ("Get lines:\n");    /* allocate ptrcount pointers */
    if (!(buffer = malloc (sizeof *buffer * ptrcount))) {
        fprintf (stderr, "error: virtual memory exhausted.\n");
        exit (EXIT_FAILURE);
    }

    while (fgets (buf, MAX_LINE_SIZE, stdin)) {
        size_t len = strlen (buf);
        if (len < 2) continue;      /* skip empty lines     */
        buf[len - 1] = 0;           /* strip trailing '\n'  */
        buffer[i++] = strdup (buf); /* allocate/copy buf    */

        if (i == ptrcount) {  /* pointer limit reached, realloc */
            void *tmp = realloc (buffer, 2 * ptrcount * sizeof *buffer);
            if (!tmp) {
                fprintf (stderr, "error: virtual memory exhausted.\n");
                break;
            }
            buffer = tmp;   /* assign tmp to buffer */
            ptrcount *= 2;  /* increase limit count */
        }
    }
    nlines = i;

    for (i = 0; i < nlines; i++) /* print the array */
        printf (" line[%2d] : %s\n", i, buffer[i]);

    for (i = 0; i < nlines; i++) /* free allocated memory */
        free (buffer[i]);
    free (buffer);

    return 0;
}

(note: since strdup allocates memory, you should check its result is not NULL, just as you do with malloc or realloc, that is left to you)

Example Input

$ cat data.txt
a quick brown fox jumps over the lazy dog.
my dog has fleas.
my cat does too.
and the fleas are colored.
red, white, and blue.

Example Use/Output

$ ./bin/chararray <data.txt
Get lines:
 line[ 0] : a quick brown fox jumps over the lazy dog.
 line[ 1] : my dog has fleas.
 line[ 2] : my cat does too.
 line[ 3] : and the fleas are colored.
 line[ 4] : red, white, and blue.

Memory/Error Check

If you allocate memory, you should always check its use with valgrind or similar memory error checking tool for your OS. You should confirm that all allocated memory has been freed and that there are no memory errors. It is simple to use, so there is no excuse not skip this step.

$ valgrind ./bin/chararray < data.txt
==21344== Memcheck, a memory error detector
==21344== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==21344== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==21344== Command: ./bin/chararray
==21344==
Get lines:
 line[ 0] : a quick brown fox jumps over the lazy dog.
 line[ 1] : my dog has fleas.
 line[ 2] : my cat does too.
 line[ 3] : and the fleas are colored.
 line[ 4] : red, white, and blue.
==21344==
==21344== HEAP SUMMARY:
==21344==     in use at exit: 0 bytes in 0 blocks
==21344==   total heap usage: 6 allocs, 6 frees, 255 bytes allocated
==21344==
==21344== All heap blocks were freed -- no leaks are possible
==21344==
==21344== For counts of detected and suppressed errors, rerun with: -v
==21344== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)

Answer 2

First of all it is better to use fgets function instead of gets which is deprecated because its bugs. see this question for more information.
After that you must use EOF (ctrl + D) in order to stop your loop because fgets read empty line that consists of just linebreak. in your code you use *q which is wrong because according to gets manual:

gets() returns s on success, and NULL on error or when end of file occurs while no characters have been read.

so cannot dereference q. I changed your code to following code and it works well on my computer:

int main(){
    char **buffer;
    int *buffer_size;
    int *line_size;
    int s,t;
    int i = 0;
    char *q;

    s = INITIAL_BUFFER_SIZE;
    buffer_size = &s;

    t = MAX_LINE_SIZE;
    line_size = &t;

    printf("Get lines:\n");
    buffer = malloc(sizeof(char *) * *buffer_size);
    buffer[0] = malloc(sizeof(char) * *line_size);
    while (1) {
        q = gets(buffer[i]);
        if (q == NULL) {
            i = i - 1;
            *buffer[i + 1] = '\0';
            break;
        }
        i++;
        buffer[i] = malloc(sizeof(char)**line_size);
        printf("%d%s\n", i, buffer[i]);
    }
}