Sorting lines

Im trying to write a program that reads a txt file and copy's all the lines in an array of strings and then sorts the strings by the length and then outputs the array of sorted strings...It compiles error free, but it just fails to run.It just freezes...It 100% something I messed up with pointers.... :confused:

Code:

---

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
void sortLines(char *line[]);
void printLines(char *line[]);

int main(int argc,char **argv){

        char *lines[200]; //200 pointers on lines
        char buffer[100]; // max length of the line
        int i=0;

        FILE *fp;

        if((fp=fopen("file.txt","r"))==NULL)
        {
                printf("File openinng error!\n");
                exit(1);
        }

        while((i<200)&&(1==fscanf(fp,"%s",buffer)))
        {
                lines[i]=malloc(strlen(buffer)+1); /*we allocate space for lines*/
                strcpy(lines[i],buffer); /*copy the buffer into that space*/
                i++;
        }

  sortLines(lines);
  printLines(lines);

    fclose(fp);
        return 0;

}

void sortLines(char *line[])
{
    int j=0;
        char *temp;
        int i;
        while((j<200) && (line[j]!=NULL)) /*if there is less than 200 lines allocated, we have to checkl if the pointer is NULL too*/
        {
                  for(i=0;i<200;i++)
                  {
                          if((strlen(line[i])) < (strlen(line[j])))
                          {
                                  temp=line[i];
                                  line[i]=line[j];
                                  line[j]=temp;
                          }
                  }
                 
                  j++;
        }
}

void printLines(char *line[]){
        int j=0;
        while((j<200)&&(line[j]!=NULL))
        {
     
        printf("%s\n",line[j]);
                free(line[j]);  /*after we print out the line we free the pointer*/
                j++;
        }
}

---

It runs for me, but it would depend on your input. A file less than 200 "lines" (see below) will cause an uninitialised pointer deference. A file with a sequence of more than 100 non-whitespace characters will cause a buffer overflow.

For a start, an fscanf on "%s" won't read a line, it will read non-whitespace characters until it finds a whitespace character.

Secondly, you are not protecting yourself from a buffer overflow, so any time you find a sequence of more than 100 non-whitespace characters, you'll overflow your buffer array. The easiest way to read a single line at a time is probably with fgets.

Thirdly, you appear to be checking lines[i] for NULL in your print function, but where do you set the last line to null when building the array?

Fourthly, in your sort function, you check for lines[j] to be NULL, but still go from 0-199 in your inner loop (i) regardless. Also, why not just use qsort instead of the O(n^2) sort you're doing now.

Quote:

---

Originally Posted by cwr

It runs for me, but it would depend on your input. A file less than 200 "lines" (see below) will cause an uninitialised pointer deference. A file with a sequence of more than 100 non-whitespace characters will cause a buffer overflow.

For a start, an fscanf on "%s" won't read a line, it will read non-whitespace characters until it finds a whitespace character.

Secondly, you are not protecting yourself from a buffer overflow, so any time you find a sequence of more than 100 non-whitespace characters, you'll overflow your buffer array. The easiest way to read a single line at a time is probably with fgets.

Thirdly, you appear to be checking lines[i] for NULL in your print function, but where do you set the last line to null when building the array?

Fourthly, in your sort function, you check for lines[j] to be NULL, but still go from 0-199 in your inner loop (i) regardless. Also, why not just use qsort instead of the O(n^2) sort you're doing now.

---

To explain:

#1: Well, I had an intention to fix the buffer overflow and number of lines check...but I just need something that works for a start..I was aware of the fact you pointed to me.

#2:Thanks for clearing that man..I will use fgets.

#3: I didn't set the last line to NULL. I just had this crazy assumption that the pointer that hasn't got allocated space is set to NULL. If num of lines is less than 200 my while function that allocates pointers stops.

#4: Oh yes, I see that, Lool... The reason I didn't use qsort is:
1. because Im a noob
2.because I would like to learn some algorithm, but please do
show me how qsort is used.

And thanks for your deep look at my code :) I appreciate it.

Using fgets like you said now works. I will now do the buffer overflow an line checks and look on sorting using qsort.

#3: When you declare something inside a block, it's an automatic variable, the values are undefined until you initialise them. You must either initialise the whole array to null:

Code:

---

char *lines[200] = { 0 };

---

or set the final pointer to NULL after you read from the file.

#4: Here is an example (from google). That example sorts an int array numerically, but you should be able to adapt the comparison function to compare lengths.

thanks man! Great help! :D i would give you points, but this forum doesn't have that feature :D