0

I have this piece of code outside the main function

mystr * arrstr[] = {
    "rec",
    "cent",
    "ece",
    "ce",
    "recent",
    "nt",
};

I modified it so that it can read the values from a text file. for this purpose i modified this working code to read line from file into array named string.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(void) {
    int i=0,j;
    char* string[100];
    char line[100];
    FILE *file; 
    file = fopen("patt", "r"); 
    while(fgets(line, sizeof(line), file)!=NULL) {
        printf("%s", line);
        string[i] = (char*)malloc(strlen(line));
        strcpy(string[i], line);
        i++;
    }
    fclose(file);
    return 0;
}

so the final code is now something like this.

..
..
char *getpatterns(const char *filename) {
    int i=0;
    char* string[100];
    char line[100];
    FILE *file; 
    file = fopen(filename, "r"); 
    while(fgets(line, sizeof(line), file)!=NULL) {
        //printf("%s", line);
        string[i] = (char*)malloc(strlen(line));
        strcpy(string[i], line);
        i++;
    }
    fclose(file);
    return(string);
}
mystr * arrstr[] = getpatterns("patt");/*{
    "rec",
    "cent",
    "ece",
    "ce",
    "recent",
    "nt",
};*/
..
..

But i get errors like this.

example1.c: In function ‘getpatterns’:
example1.c:43:2: warning: return from incompatible pointer type [enabled by default]
example1.c:43:2: warning: function returns address of local variable [enabled by default]
example1.c: At top level:
example1.c:45:1: error: invalid initializer
make: *** [example1.o] Error 1

Here line 45 is this line

mystr * arrstr[] = getpatterns("patt");/*{

Please suggest corrective action.

5
  • You can initialize static storage (globals) only with constant expressions. Commented Jan 20, 2013 at 18:24
  • @StoryTeller sorry i did not get you. thanks for replying Commented Jan 20, 2013 at 18:28
  • Quite simple, really. The result of the call to getpatterns cannot be known until run time. Therefore it is not a constant initializer (one that can be known at compile time). Other than that, you should really heed to compiler warnings. Commented Jan 20, 2013 at 18:32
  • There are three error messages for three separate errors. Have you tried to parse any of them? How far have you got? There is a fourth error, a buffer overrun around calls to strlen and malloc, that is present in both versions and does not provoke a compiler message. Commented Jan 20, 2013 at 18:35
  • i am stuck, i can handle the malloc issues i guess but not these. Commented Jan 20, 2013 at 18:37

1 Answer 1

2

The first warnings are that you are trying to return a char ** as a char * (which is not a good idea), and that you are returning a local variable which is deallocated when the function returns (also not a good idea). The last is telling you that you can't use function calls in initializers of global variables in C (you can do some of that in C++, though I'm not convinced you can do this one).

Fixing it will take some rethinking. You need the function to return allocated memory, or you need to pass the memory to the function. And you'll have to change the type of the global variable. And you'll need to know how many entries there are in the array, somehow.

mystr **arrstr = 0;  // Either
mystr  *arrstr[100]; // Or

On the whole, I'd probably go with memory allocation and the 'either' declaration:

mystr **arrstr = 0;

char **getpatterns(const char *file)
{
    char **array = 0;
    ...code similar to yours that allocates entries in the array...
    ...include space for a null pointer to mark the end of the list of strings...
    return(array);
}

int main(void)
{
    arrstr = getpatterns("patt");
    ...
}

(Another 'cheat' mechanism would use static char *string[100]; in getpatterns(); you still have to fix the return type and the type of the global variable.)


I tried these but, errors were not resolved: ...

It's impossible to tell exactly what was wrong without your code. However, the code below works for me. The source code was in a file gp.c; the source code prints itself, and releases the memory. Checked under valgrind with a clean bill of health.

Note that your original code did not allocate enough space for the strings it was copying (because you retained the newline read by fgets() — but you were at least using fgets() and not gets(), which is very important). This code uses memmove() — it could use memcpy() instead since there's guaranteed to be no overlap, but memmove() always works and memcpy() doesn't necessarily work when the source data overlaps the target data. It knows how long the string is, so the copy function doesn't need to test for whether the character being copied is a NUL '\0'. The code carefully ensures that there's a null pointer at the end of the list of pointers; that's how you know when you've reached the end of the list of strings. The code also works when gp.c is an empty file.

The algorithm using three items num_xxx, max_xxx, and xxx is a typical way to handle incremental allocation. It typically over-allocates slightly; if you're concerned about the space, you could use strings = realloc(strings, (num_strings+1) * sizeof(*strings)); max_strings = num_strings + 1; at the end of the loop to release the extra space. The + 1 is to allow for the null pointer. By roughly doubling the size allocated each time you allocate, you avoid quadratic behaviour compared with incrementing by one each time.

Notice too that the code carefully avoids losing the allocated space if the realloc() fails. You should 'never' use space = realloc(space, new_size); to avoid losing your pointer. The code carefully avoids dereferencing null pointers, and simply stops reading when there is a memory shortage.

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

extern char **getpatterns(const char *filename);

char **getpatterns(const char *filename)
{
    size_t num_strings = 0;
    size_t max_strings = 0;
    char **strings = 0;
    FILE *file = fopen(filename, "r"); 

    if (file != 0)
    {
        char line[4096];
        while (fgets(line, sizeof(line), file) != NULL)
        {
            if (max_strings == 0 || num_strings >= max_strings - 1)
            {
                size_t   new_num = max_strings * 2 + 2;
                char   **new_space = realloc(strings, new_num * sizeof(*new_space));
                if (new_space == 0)
                    break;
                strings = new_space;
                max_strings = new_num;
            }
            size_t len = strlen(line);  /* Includes '\n' at end */
            strings[num_strings] = (char*)malloc(len);
            memmove(strings[num_strings], line, len - 1);
            strings[num_strings][len] = '\0';
            strings[++num_strings] = 0; /* Null terminate list of strings */
        }
        fclose(file);
    }
    return(strings);
}

int main(void)
{
    char **data = getpatterns("gp.c");
    char **argp = data;

    if (argp != 0)
    {
        /* Print data */
        while (*argp != 0)
            puts(*argp++);

        /* Free space */
        argp = data;
        while (*argp != 0)
            free(*argp++);
        free(data);
    }

    return(0);
}
Sign up to request clarification or add additional context in comments.

3 Comments

thanks , i will try to include your suggestions and see if i can make it work.
i tried these but, errors were not resolved ->example1.c: In function ‘getpatterns’: example1.c:38:13: warning: assignment from incompatible pointer type [enabled by default] example1.c:39:3: warning: passing argument 1 of ‘strcpy’ from incompatible pointer type [enabled by default] /usr/include/string.h:128:14: note: expected ‘char * restrict’ but argument is of type ‘char **’ example1.c:43:2: warning: return from incompatible pointer type [enabled by default] example1.c:43:2: warning: function returns address of local variable [enabled by default] example1.c: At top level: and many more
thank you very much Jonathan for your efforts to help me resolve my problems with programming. Thanks a lot.

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.