From: fred AT genesis DOT demon DOT co DOT uk (Lawrence Kirby)
Newsgroups: comp.lang.c,comp.os.msdos.djgpp
Subject: Re: array of pointers to strings, BC++, DJGPP and weird printing behavior
Date: Wed, 06 Nov 96 13:38:07 GMT
Organization: none
Lines: 143
Message-ID: <847287487snz@genesis.demon.co.uk>
References: <55nrvs$sdo AT solaris DOT cc DOT vt DOT edu>
Reply-To: fred AT genesis DOT demon DOT co DOT uk
To: djgpp AT delorie DOT com
DJ-Gateway: from newsgroup comp.os.msdos.djgpp

In article <55nrvs$sdo AT solaris DOT cc DOT vt DOT edu>
           namille2 AT vt DOT edu "Matthew Miller" writes:

>Hello,
>
>I have got the array of pointers to strings concept down,
>but now (I think) something weird is happening.  This code
>snippet (along w/ a main function) compiles without errors
>on both BC++ 3.1 and DJGPP. But, the executables don't act
>the same.  With DJGPP, running the exe. produces an endless loop
>of seg. faults. Also, the file that is opened is just a plain
>text file containing one word per line
>
>While, when compiled w/ BC++ the program executes fine, but,
>if you look a the source code there are two statements to
>print the array of strings; one is inside the do while loop
>(there is comment above it) and another right after the loop.
>This is where the odd behavior comes in, printing the strings
>from within the loop goes fine, no problem.  While, outside
>the loop when the strings are printed there is a few garbage
>characters tagged at the end of the string.  Why is there this
>difference? The same statements are used to print the strings,
>but the output is different depending if printed from inside
>the loop or outside.
>
>Path:     <- printed from inside the loop
>Path:    <- printed from outside the loop
>
>Can someone help with this, exp. help me to get this code to
>work with DJGPP.
>
>Thanks.  Matthew Miller
>
>-------- the below is the offending code ---------
>#include <stdio.h>
>#include <stdlib.h>
>#include <assert.h>
>
>#define MAX_KWLENGTH 40 /* THE MAX. LENGTH OF THE KEYWORD */
>#define INI_NAME "ngformat.ini" /* NAME OF THE KEYWORD FILE */
>
>void ReadIni ( void ) {
>        char **KeyWords;
>        int index = 0, WordNumber = 0, j;
>        FILE *ini;
>        char string[ MAX_KWLENGTH ], *ret, *key[ MAX_KWLENGTH ];
>        
>/* ALLOCAT THE FIRST POINTER TO THE KEYWORD STRING */           
>        KeyWords = (char **) malloc ( sizeof(char) );

You're only allocating enough space for a char, not a pointer (char * in this
case). Make that:

         KeyWords = malloc ( sizeof(char *) );

or better style:

         KeyWords = malloc ( sizeof *KeyWords );

You're better off without the cast. All it does is clutter the code and
possibly prevent the compile giving you important diagnostics (such as
not having included stdlib.h).

>        assert ( KeyWords != NULL );

You shouldn't really use assert() for this, It is a debugging tool. malloc()
returning NULL is a runtime error condition ans should have permanent code
to check for it, as you do for fopen() below.

>        ini = fopen ( INI_NAME, "r" );
>        if ( ini == NULL ) {
>                fprintf ( stderr, "\nReadIni Error!! -- Unable to open %s.\n", 
>INI_NAME );
>                exit ( EXIT_FAILURE );
>        }
>        

>        do {            
>                ret = fgets ( string, MAX_KWLENGTH, ini );
>                if ( ret != NULL ) {

I suggest you write this as simply:

         while ( fgets ( string, MAX_KWLENGTH, ini ) != NULL ) {

And change the end of the loop appropriately.

>                        if ( WordNumber != 0 ) {
>                                /* ALLOCAT THE NEXT POINTER TO THE KEYWORD
> STRING 
>*/              
>                                KeyWords = (char **) realloc ( KeyWords, 
>sizeof(char) );

Here you're telling realloc to change the size of the allocated space
to a single char. Presumably you wanted an array of char * pointers. How
many I'm not sure. You're keeping WordNumber as a count here but using
index below to index the array. In general you want:

     KeyWords = realloc ( KeyWords, nelems * sizeof(*KeyWords) );

where nelems is the number of elements you want the array to have after
the realloc call is complete. Note again you should lose the cast.
Normally you shouldn't assign back to the variable you pass to realloc
since you get a memory leak if realloc fails. Since you exit immediately
in this case isn't a big problem.

>                                assert ( *KeyWords != NULL );
>                                ++WordNumber;                           
>                                }
>
>                        *key = (char*) strtok ( string, "\n" );   /* STRIP
> NEWLINE 
>*/

That was bandied around as a "neat trick" for a while. However it doesn't
work for blank lines (i.e. lines that consist of only a newline character).

>                        KeyWords[ index ] = (char *) malloc ( 
>strlen(*key)*sizeof(char) );

You need to reserve space for the terminating null character of a string.
Also sizeof(char) is, by definition, 1 so :

                         KeyWords[ index ] = malloc ( strlen(*key) + 1 );

>                        assert( KeyWords[ index ] != NULL );
>                        strcpy( (char*) KeyWords[index], *key );

KeyWords[index] already has type char*. All the cast does is prevent the
compiler warning you about mistakes in your code - very bad idea.

>/* First print statement */
>                        printf ( "%s\n", (char*)KeyWords[ index ] ); 

Again, lose the cast.

-- 
-----------------------------------------
Lawrence Kirby | fred AT genesis DOT demon DOT co DOT uk
Wilts, England | 70734 DOT 126 AT compuserve DOT com
-----------------------------------------