Reputation: 13

How to store arrays inside array of pointers

i'm trying to implement little program that takes a text and breaks it into lines and sort them in alphabetical order but i encountered a little problem, so i have readlines function which updates an array of pointers called lines, the problem is when i try to printf the first pointer in lines as an array using %s nothing is printed and there is no errors.

I have used strcpy to copy an every single text line(local char array) into a pointer variable and then store that pointer in lines array but it gave me the error.

Here is the code:

#include <stdio.h>

#define MAXLINES 4
#define MAXLENGTH 1000

char *lines[MAXLINES];

void readlines() {
    int i;
    for (i = 0; i < MAXLINES; i++) {
        char c, line[MAXLENGTH]; 
        int j;
 
        for (j = 0; (c = getchar()) != '\0' && c != '\n' && j < MAXLENGTH; j++) {
            line[j] = c;
        }

        lines[i] = line;
    }
}

int main(void) {
    readlines();
    printf("%s", lines[0]);
    getchar();

    return 0;
}

Upvotes: 0

Answers (3)

arfneto

Reputation: 1765

Problem 1

char *lines[MAXLINES];

For the compiler it makes no difference how you write this, but for you, as you are learning C, maybe it is worth consider different spacing and naming. Question is: what is lines[]? lines[] is supposed to be an array of strings and hold some text inside. So lines[0] is a string, lines[1] is a string and so on. As pointed in a comment you could also use char lines[MAX_LINES][MAX_LENGTH] and have a 2D box of NxM char. This way you would have a pre-determined size in terms of number and size of lines and have simpler things at a cost of wasting space in lines of less than MAX_LENGTH chars and having a fixed number of lines you can use, but no need to allocate memory.
A more flexible way is to use an array of pointers. Since each pointer will represent a line, a single one

        char* line[MAXLINES];

is a better picture of the use: line[0] is char*, line[1] is char* and so on. But you will need to allocate memory for each line (and you did not) in your code.

Remember int main(int argc, char**argv) This is the most flexible way, since in this way you can hold any number of lines. The cost? Additional allocations.

    size_t n_lines;
    char** line;

This may be the best representation, as known by every C program since K&R.

Problem 2

  for (
    j = 0;
    (c = getchar()) != '\0' && c != '\n' && j < MAXLENGTH;
    j++) {
       line[j] = c;
        }
    lines[i] = line;

This loop does not copy the final 0 that terminates each string. And reuses the same line, a char[] to hold the data as being read. And the final line does not copy a string, if one existed there. There is no one since the final 0 was stripped off by the loop. And there is no data too, since the area is being reused.

A complete C example of `uploading` a file to a container in memory

I will let an example of a more controlled way of writing this, a container for a set of lines and even a sorting function.

a data structure

The plan is to build an array of pointers as the system does for main. Since we do no know ahead the number of lines and do not want this limitation we will allocate memory in groups of blk_size lines. At any time we have limit pointers to use. From these size are in use. line[] is char* and points to a single line of text. The struct is

typedef struct
{
    size_t blk_size;  // block
    size_t limit;     // actual allocated size
    size_t size;      // size in use
    char** line;      // the lines

}   Block;

the test function

Block* load_file(const char*);

Plan is to call load_file("x.txt") and the function returns a Block* pointing to the array representing the lines in file, one by one. Then we call qsort() and sort the whole thing. If the program is called lines we will run

    lines x.txt

and it will load the file x.txt, show its contents on screen, sort it, show the sorted lines and then erase everything at exit.

`main()` for the test

int    main(int argc, char** argv)
{
    char msg[80] = {0};
    if (argc < 2) usage();
    Block* test = load_file(argv[1]);
    sprintf(msg, "==> Loading \"%s\" into memory", argv[1]);    
    status_blk(test, msg);
    qsort(test->line, test->size, sizeof(void*), cmp_line);
    sprintf(msg, "==> \"%s\" after sort", argv[1]);
    status_blk(test, msg);
    test = delete_blk(test);
    return 0;
};

As planned

load_file() is the constructor and load the file contents into a Block.
status_blk() shows the contents and accepts a convenient optional message
qsort() sorts the lines using a one-line cmp_line() function.
status_blk() is called again and shows the now sorted contents
as in C++ delete_blk() is the destructor and erases the whole thing._

output using `main()` as `tlines.c` for testing

PS M:\> .\lines tlines.c
loading "tlines.c" into memory
Block extended for a total of 16 pointers


==> Loading "tlines.c" into memory
Status: 13 of 16 lines. [block size is 8]:
   1    int main(int argc, char** argv)
   2    {
   3        char msg[80] = {0};
   4        if (argc < 2) usage();
   5        Block* test = load_file(argv[1]);
   6        sprintf(msg, "==> Loading \"%s\" into memory", argv[1]);
   7        status_blk(test, msg);
   8        qsort(test->line, test->size, sizeof(void*), cmp_line);
   9        sprintf(msg, "==> \"%s\" after sort", argv[1]);
  10        status_blk(test, msg);
  11        test = delete_blk(test);
  12        return 0;
  13    };

==> "tlines.c" after sort
Status: 13 of 16 lines. [block size is 8]:
   1        Block* test = load_file(argv[1]);
   2        char msg[80] = {0};
   3        if (argc < 2) usage();
   4        qsort(test->line, test->size, sizeof(void*), cmp_line);
   5        return 0;
   6        sprintf(msg, "==> Loading \"%s\" into memory", argv[1]);
   7        sprintf(msg, "==> \"%s\" after sort", argv[1]);
   8        status_blk(test, msg);
   9        status_blk(test, msg);
  10        test = delete_blk(test);
  11    int main(int argc, char** argv)
  12    {
  13    };

About the code

I am not sure if it needs much explanation, it is a single function that does the file loading and it has around 20 lines of code. The other functions has less than 10. The whole file is represented in line that is char** and Block has the needed info about actual size.

Since line[] is an array of pointers we can call

        qsort(test->line, test->size, sizeof(void*), cmp_line);

and use

int cmp_line(const void* one, const void* other)
{
    return strcmp(
        *((const char**)one), *((const char**)other));
}

using strcmp() to compare the strings and have the lines sorted.

create_blk() accepts a block size for use in the calls to realloc() for eficiency.
Delete a Block is a 3-step free() in the reverse order of allocation.

The complete code

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

typedef struct
{
    size_t blk_size;  // block
    size_t limit;     // actual allocated size
    size_t size;      // size in use
    char** line;      // the lines

}   Block;

Block* create_blk(size_t);
Block* delete_blk(Block*);
int    status_blk(Block*, const char*);


Block* load_file(const char*);
int    cmp_line(const void*, const void*);
void   usage();

int    main(int argc, char** argv)
{
    char msg[80] = {0};
    if (argc < 2) usage();
    Block* test = load_file(argv[1]); 
    sprintf(msg, "\n\n==> Loading \"%s\" into memory", argv[1]);    
    status_blk(test, msg);
    qsort(test->line, test->size, sizeof(void*), cmp_line);
    sprintf(msg, "\n\n==> \"%s\" after sort", argv[1]);
    status_blk(test, msg);
    test = delete_blk(test);
    return 0;
};

int cmp_line(const void* one, const void* other)
{
    return strcmp(
        *((const char**)one), *((const char**)other));
}

Block* create_blk(size_t size)
{
    Block* nb = (Block*)malloc(sizeof(Block));
    if (nb == NULL) return NULL;
    nb->blk_size = size;
    nb->limit    = size;
    nb->size     = 0;
    nb->line     = (char**)malloc(sizeof(char*) * size);
    return nb;
}

Block* delete_blk(Block* blk)
{
    if (blk == NULL) return NULL;
    for (size_t i = 0; i < blk->size; i += 1)
        free(blk->line[i]);  // free lines
    free(blk->line);         // free block
    free(blk);               // free struct
    return NULL;
}

int status_blk(Block* bl,const char* msg)
{
    if (msg != NULL) printf("%s\n", msg);
    if (bl == NULL)
    {
        printf("Status: not allocated\n");
        return -1;
    }
    printf(
        "Status: %zd of %zd lines. [block size is %zd]:\n",
        bl->size, bl->limit, bl->blk_size);
    for (int i = 0; i < bl->size; i += 1)
        printf("%4d\t%s", 1 + i, bl->line[i]);
    return 0;
}

Block* load_file(const char* f_name)
{
    if (f_name == NULL) return NULL;
    fprintf(stderr, "loading \"%s\" into memory\n", f_name);
    FILE* F = fopen(f_name, "r");
    if (F == NULL) return NULL;
    // file is open
    Block* nb = create_blk(8); // block size is 8
    char   line[200];
    char*  p = &line[0];
    p        = fgets(p, sizeof(line), F);
    while (p != NULL)
    {
        // is block full?
        if (nb->size >= nb->limit)
        {
            const size_t new_sz = nb->limit + nb->blk_size;
            char*        new_block =
                realloc(nb->line, (new_sz * sizeof(char*)));
            if (new_block == NULL)
            {
                fprintf(
                    stderr,
                    "\tCould not extend block to %zd "
                    "lines\n",
                    new_sz);
                break;
            }
            printf(
                "Block extended for a total of %zd "
                "pointers\n",
                new_sz);
            nb->limit = new_sz;
            nb->line  = (char**)new_block;
        }
        // now copy the line
        nb->line[nb->size] = (char*)malloc(1 + strlen(p));
        strcpy(nb->line[nb->size], p);
        nb->size += 1;
        // read next line
        p = fgets(p, sizeof(line), F);
    };  // while()
    fclose(F);
    return nb;
}

void usage()
{ 
    fprintf(stderr,"Use: program file_to_load\n");
    exit(EXIT_FAILURE);
}

Upvotes: 0

Andreas Wenzel

Reputation: 24726

One problem is the following line:

lines[i] = line;

In this line, you make lines[i] point to line. However, line is a local char array whose lifetime ends as soon as the current loop iteration ends. Therefore, lines[i] will contain a dangling pointer (i.e. a pointer to an object that is no longer valid) as soon as the loop iteration ends.

For this reason, when you later call

printf("%s", lines[0]);

lines[0] is pointing to an object whose lifetime has ended. Dereferencing such a pointer invokes undefined behavior. Therefore, you cannot rely on getting any meaningful output, and your program may crash.

One way to fix this would be to not make lines an array of pointers, but rather an multidimensional array of char, i.e. an array of strings:

char lines[MAXLINES][MAXLENGTH+1];

Now you have a proper place for storing the strings, and you no longer need the local array line in the function readlines.

Another issue is that the line

printf("%s", lines[0]);

requires that lines[0] points to a string, i.e. to an array of characters terminated by a null character. However, you did not put a null character at the end of the string.

After fixing all of the issues mentioned above, your code should look like this:

#include <stdio.h>

#define MAXLINES 4
#define MAXLENGTH 1000

char lines[MAXLINES][MAXLENGTH+1];

void readlines() {
    int i;
    for (i = 0; i < MAXLINES; i++) {
        char c; 
        int j;
 
        for (j = 0; (c = getchar()) != '\0' && c != '\n' && j < MAXLENGTH; j++) {
            lines[i][j] = c;
        }

        //add terminating null character
        lines[i][j] = '\0';
    }
}

int main(void) {
    readlines();
    printf("%s", lines[0]);

    return 0;
}

However, this code still has a few issues, which are probably unrelated to your immediate problem, but could cause trouble later:

The function getchar will return EOF, not '\0', when there is no more data (or when an error occurred). Therefore, you should compare the return value of getchar with EOF instead of '\0'. However, a char is not guaranteed to be able to store the value of EOF. Therefore, you should store the return value of getchar in an int instead. Note that getchar returns a value of type int, not char.
When j reaches MAX_LENGTH, you will call getchar one additional time before terminating the loop. This can cause undesired behavior, such as your program waiting for more user input or an important character being discarded from the input stream.

In order to also fix these issues, I recommend the following code:

#include <stdio.h>

#define MAXLINES 4
#define MAXLENGTH 1000

char lines[MAXLINES][MAXLENGTH+1];

void readlines() {
    int i;
    for (i = 0; i < MAXLINES; i++)
    {
        //changed type from "char" to "int"
        int c;

        int j;

        for ( j = 0; j < MAXLENGTH; j++ )
        {
            if ( (c = getchar()) == EOF || c == '\n' )
                break;

            lines[i][j] = c;
        }

        //add terminating null character
        lines[i][j] = '\0';
    }
}

int main(void) {
    readlines();
    printf("%s", lines[0]);

    return 0;
}

Upvotes: 3

Fe2O3

Reputation: 8344

Try something like this:

#include <stdio.h>
#include <stdlib.h> // for malloc(), free(), exit()
#include <string.h> // for strcpy()

#define MAXLINES 4
#define MAXLENGTH 1000
char *lines[MAXLINES];

void readlines() {
    for( int i = 0; i < MAXLINES; i++) {
        char c, line[MAXLENGTH + 1]; // ALWAYS one extra to allow for '\0'
        int j = 0; 

        // RE-USE(!) local array for input characters until NL or length
        // NB: Casting return value to character (suppress warning)
        while( (c = (char)getchar()) != '\0' && c != '\n' && j < MAXLENGTH )
            line[ j++ ] = c;

        line[j] = '\0'; // terminate array (transforming it to 'string')

        // Attempt to get a buffer to preserve this line
        // (Old) compiler insists on casting return from malloc()
        if( ( lines[i] = (char*)malloc( (j + 1) * sizeof lines[0][0] ) ) == NULL ) {
            fprintf( stderr, "malloc failure\n" );
            exit( -1 );
        }
        strcpy( lines[i], line ); // preserve this line
    }
}

int my_main() {

    readlines(); // only returns after successfully reading 4 lines of input

    for( int i = 0; i < MAXLINES; i++)
        printf( "Line %d: '%s'\n", i, lines[i] ); // enhanced

    /* Maybe do stuff here */

    for( int j = 0; j < MAXLINES; j++) // free up allocated memory.
        free( lines[j] );

    return 0;
}

If you would prefer to 'factor out` some code (and have a facility that you've written is absent, here's a version:

char *my_strdup( char *str ) {
    int len = strlen( str )  + 1; // ALWAYS +1

    // Attempt to get a buffer to preserve this line
    // (Old) compiler insists on casting return from malloc()
    char *pRet = (char*)malloc( len * sizeof *pRet );

    if( pRet == NULL ) {
        fprintf( stderr, "malloc failure\n" );
        exit( -1 );
    }
    return strcpy( pRet, str );
}

The the terminating and preserve is condensed to:

    line[j] = '\0'; // terminate array (transforming it to 'string')
    lines[i] = my_strdup( line ); // preserve this line

Upvotes: -1