Quicksort on strings and file in C language

Question

I have a question about the implementation of my Quicksort on strings in a file.

This exercise requires to input the file name of the file which you want to open and it can have at most 100 items. With the function char filling(FILE* myfile), every row contained in the file is copied into an array through a while cycle which ends at the End Of File.

Then it is called the function sort(strings, start, end, leq) to sort the array of strings which, at the end of the function, will be copied into a new file called sorted_myfile.

My question is: how can I test if my program works with files?

In the function char filling the declaration leq_fn leq refers to the declaration of a pointer to function typedef bool(* leq_fn )(char *, char *) which is included in the function of the quicksort. I hope I gave you all the information you need to help me. Thank you.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "quicksort.h"
#define MAX_LENGTH 100
#define EXTRA_LENGTH 101
#define LENGTH_STRING 255

char filling(FILE *myfile);
char *strings[MAX_LENGTH]; //Array of at most 100 items

char filling(FILE *myfile){
    char *row;
    char new_filename[LENGTH_STRING];
    FILE* sorted_myfile;
    int i=0, start=0, end=MAX_LENGTH-1;
    bool res_cmplen, res_cmpalpha, res_cmpalpha_nocase;
    FILE *sorted_myfile;

   while(i<=EOF){ 
     if(i==EXTRA_LENGTH){
        printf("The file has more than 100 items\n");
     }else{
        row = calloc(LENGTH_STRING, sizeof(char));
        fgets(row, LENGTH_STRING, myfile);
        strings[i]=row;
        i++;
        free(row);
    }
   }
   leq_fn leq;

   leq = cmp_len;
   res_cmplen = (*leq)("hello", "bye");
   leq = cmp_alpha;
   res_cmpalpha = (*leq)("HELLO", "bye");
   leq = cmp_alpha_nocase;
   res_cmpalpha_nocase = (*leq)("hello", "BYE");

   sort(strings, start, end, leq);
   scanf("%s", new_filename);
   sorted_myfile = fopen(new_filename, "w");
   for(i=0;i<end;i++){
     fputs(strings[i], sorted_myfile);
   }

   return 0;
}


int main(void) {
     char filename[LENGTH_STRING]; 
     FILE *myfile;

     printf("What is the file name?\n");
     scanf("%s", filename);
     myfile =  fopen(filename, "r");

     if(ferror(myfile)!=0){
       printf("The file doesn't exist\n");
     }else{
       filling(myfile);
    }

  return 0;
}

In the filling function, the loop will not be iterated even once. The macro EOF is usually defined as -1, and 0 <= -1 is never true. Also, to check if the file opened correctly, you need to check if the returned pointer is NULL or not, not call ferror, and there might also be many other problems besides the file not existing. — Some programmer dude
– Some programmer dude, Commented Jun 1, 2015 at 10:58
@JoachimPileborg so in the while cycle the variable i would be initialized at -1? — Daniela Favia
– Daniela Favia, Commented Jun 1, 2015 at 11:01
Also, you have undefined behavior in your code, you first allocate memory using calloc, copy the pointer (but not what the pointer points to) and then free the just allocated memory, leaving the pointer to point to unallocated memory. Using that pointer in any way leads to undefined behavior. — Some programmer dude
– Some programmer dude, Commented Jun 1, 2015 at 11:03
No you should loop while i is less than MAX_LENGTH (the size of the strings array). And also check what fgets return, so you don't iterate once you reach the end of the file. — Some programmer dude
– Some programmer dude, Commented Jun 1, 2015 at 11:04
Is the working directory of the program the same as the directory where the file is? If you're using some kind of IDE (Integrated Development Environment) then you should check the project settings to make sure. — Some programmer dude
– Some programmer dude, Commented Jun 1, 2015 at 11:12

WedaPashi · Accepted Answer · 2015-06-01 12:03:48Z

I'd first start with fixing these:

Change char *row; to char *row = NULL;
Similarly change FILE* sorted_myfile; to FILE* sorted_myfile = NULL;
Change bool res_cmplen, res_cmpalpha, res_cmpalpha_nocase; to
bool res_cmplen = 0, res_cmpalpha = 0, res_cmpalpha_nocase = 0; or
bool res_cmplen = 1, res_cmpalpha = 1, res_cmpalpha_nocase = 1;
depending upon what is safer for implementation.
Change FILE *sorted_myfile; to FILE *sorted_myfile = NULL;
In main() change FILE *myfile; to FILE *myfile = NULL;
After free(row); row becomes a dangling pointer and then you use it as is, and then on the last iteration of the loop, you leave it as is, which would invoke undefined behaviour. So, do a row = NULL; after freeing the memory.
char *strings[MAX_LENGTH]; is not an //Array of at most 100 items, but an //array of MAX_LENGTH pointers. What happens here: strings[i]=row;?
In the snippet below:
```
while(i<=EOF){ 
   if(i==EXTRA_LENGTH){  
      printf("The file has more than 100 items\n");  
   }else{  
      row = calloc(LENGTH_STRING, sizeof(char));  
      fgets(row, LENGTH_STRING, myfile);  
      strings[i]=row;  
      i++;  
      free(row);  
   }
 }
```
i is incremented in else{} condition and if(i==EXTRA_LENGTH) evaluates to true i.e. if i is equal to EXTRA_LENGTH, it will not be incremented or modified, since that takes place in else{} only. So When i == 255, your code will never enter else{} condition and only print "The file has more than 100 item " infinitely.
This is a minor one, but, In main() I can see myfile = fopen(filename, "r");, this opens the file. Fine. Though I don't see when you close it, if you do at all. Missing a fclose(); somewhere may be?

6. Since the memory needs to stay allocated during the sort operation, the free should not be performed until afterwards.

Collectives™ on Stack Overflow

Quicksort on strings and file in C language

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related