malloc 1d string array

Hi all,
I'm' writing a program in which I can't get round to see what I am doing wrong in a small part of it that deals with:
-dynamically allocating a 1d string array (or 2d char array..)
-reading a file and storing each line in a string (as previously allocated)

I would be grateful if you could direct me in debugging my program. Here's the relevant pieces code:

Code:

---

#include<stdio.h>      /* for input and output */
#include<stdlib.h>      /* for memory management */
#include<string.h>      /* for memset */

int main(void) {

        unsigned int nrows=0;                  /* number of data rows in the file */
        unsigned int comment=0;                /* number of comment lines in file */
        unsigned int i=0;                          /* counters */
        const unsigned int linelen=700;        /* length of buffer line */
        FILE *fp=NULL;                          /* pointer to file to read data */
        char oldname[30];                      /* name of file to read */
        char **buff=NULL;                      /* buffer line */
        char templine[700];                    /* temporary line for initial reading of the file */

        /* open a file to read */
        printf("Enter the name of the file you want to read data from: ");
        scanf("&#37;s", oldname);
        if (!(fp=fopen(oldname,"r"))) {
                perror(oldname);
                exit(1);
        }

        /* count the number of comment lines, and data lines */
        while (fgets(templine,sizeof(templine),fp)!=NULL) {
                if (templine[0]=='#') {
                        comment++;
                } else {
                        nrows++;
                }
        }
        printf("\nThere are %u comments and %u rows", comment, nrows);

        /* allocate string array to write all the lines from the file */
        buff=malloc((comment+nrows)*sizeof(*buff));
        if (buff==NULL) {
                printf("\nCould not allocate  pointer array");
        }
        for (i=0; i<(comment+nrows); i++) {
                buff[i]=malloc(linelen*sizeof(*buff[i]));
                if (buff[i]==NULL) {
                        printf("\nCould not allocate array %u", i);
                }
        }

        /* read all lines into buffer, and print to screen */
        rewind(fp);
        for (i=0; i<(comment+nrows); i++) {
                fgets(buff[i],sizeof(buff[i]),fp);
                printf("\n%s", buff[i]);
        }

        /* clean up memory */
        fclose(fp);
        for (i=0; i<(comment+nrows); i++) {
                free(buff[i]);
        }
        free(buff);

        return 0;
}

---

EDIT: When I run the code above, I'm getting the result of every three characters to be written to each of the 1d string arrays. This looks like maybe I'm not allocating enough space for the length of my string, even though I want to set it as 700 via the "linelen" variable.

Also, I'm wondering why for array indexing, people generally use "int" and not "unsigned int"? It seems for me that "unsigned int" is more appropriate because of the +ve range being bigger, and because of no need for negative numbers when indexing an array.

Thanks!
Spiros

1) Well, for the array indexing just to avoid the "unsigned" thing. Though there are a lot of people that use the unsigned thing. Not a huge difference. Especially in C99, writing

Code:

---

if (unsigned int i=0; ....)

---

looks much bigger than the simple int i=0 :P

If you pas iterator i to a function that want int and not unsigned int then you will avoid a compiler's warning and maybe an error.
If for example you want to do something with the iterator like "sum = i*b" then it might be slightly faster to have i the same type as b.

Generally, unsigned and signed are similar but not the same! It might be easier avoiding a bug if you had it signed rather unsigned.

Not supporting anything, just note that in some cases there might be a difference and one more correct than the other.

2) I ll check your code and see in a while :)
EDIT: OK found the fault

Code:

---

change this (line 49)
fgets(buff[i],sizeof(buff[i]),fp);
with this:
fgets(buff[i],linelen*sizeof(*buff[i]),fp);

---

Also, fgets "reads" and stores the '\n' character. So change this

Code:

---

this (line 50):
printf("\n%s", buff[i]);
with this:
printf("%s", buff[i]);

---

and you are done ;)

Code:

---

buff[i]=malloc(linelen*sizeof(*buff[i]));

---

Code:

---

fgets(buff[i],sizeof(buff[i]),fp);

---

sizeof(*buff[i]) will be a size of a char, and sizeof(buff[i]) will be the size of a char *.

sizeof() doesn't do anything for you in terms of the length of a dynamically sized buffer.

that's brilliant guys, i've understood what was wrong. Thanks a lot for the explanations of the outputs of sizeof(buff[i]), and sizeof(*buff[i]).

many thanks
Spiros