-1

I'm pretty new to C and someone "challenged" me to try and create a sorting program using C. I come from languages that are higher-level where doing something like this is easier, but I guess the lower-level intricacies are way over my head. I haven't implemented the sorting yet, because I've ran across an obstacle (just one of many) along the way.

Anyways, here is the code I have so far:

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

int main(int argc, char *argv[])
{
    FILE *unsortedFile; /* prepare file variable */
    char lineBuffer[100]; /* prepare variable for each line */
    char *listOfLines[100]; /* prepare line array variable to be sorted */
    int n = 0; 
    int i;
    if (argc == 2) /* if a file has been given */
    {
        unsortedFile = fopen(argv[1], "r"); /* open it readonly */
        if (unsortedFile == NULL) /* if it couldn't open */
        {
            printf("Couldn't open the file %s\n", argv[1]);
            printf("Does it exist?\n");
            return -1; /* stop the program here, return non-zero for error */
        }
        printf("original file:\n\n");
        while (fgets(lineBuffer, sizeof(lineBuffer), unsortedFile))
        {
            printf("%s", lineBuffer);
            listOfLines[n] = lineBuffer; /* store line buffer to the array */
            n = ++n; /* increase n for the next array element */
        }
        printf("\nLines to be sorted: %d\n", n);
        for (i = 0; i < n; i++)
        {
            printf("%s", listOfLines[i]);
        }
    } else /* if no or too many args provided */
    {
        printf("\nArgument error - you either didn't supply a filename\n");
        printf("or didn't surround the filename in quotes if it has spaces\n\n");
        return -1; /* return non-zero for error */
    }
}

At this point, you're probably busy vomiting over the messiest spaghetti code you've ever seen... but anyways, the issue occurs with that while statement, I guess. The original file prints to the console fine, but I don't think each line is being stored to listOfLines.

Here is what's in file.txt, the file I am supplying as an argument to the program:

zebra
red
abacus
banana

And here is the output of the program:

dustin@DESKTOP-033UL9B:/mnt/c/Users/Dustin/projects/c/sort$ ./sort file.txt
original file:

zebra
red
abacus
banana

Lines to be sorted: 4
banana
banana
banana
banana
dustin@DESKTOP-033UL9B:/mnt/c/Users/Dustin/projects/c/sort$

Looks like the last line of the file is the only one being stored to listOfLines? What could cause this behavior?

Thanks in advance!

giggybyte
  • 1
  • 3

2 Answers2

3

listOfLines is an array of pointers. All those pointers are set to point to lineBuffer:

     listOfLines[n] = lineBuffer;

And lineBuffer is repeatedly overwritten by lines from the file. The last line is banana, which is the final value of lineBuffer.

Your code then prints the values in listOfLines, which are all pointers to lineBuffer.


This line is very wrong, by the way (it has undefined behavior):

    n = ++n;

If you want to increment n, that's either

n = n + 1;

or

++n;

Basically, don't modify the same variable twice within the same statement.

melpomene
  • 84,125
  • 8
  • 85
  • 148
1
  1. You need an array of char arrays (not array of pointers)

Switched:

char *lineOfLines[100];      // array of pointers
char listOfLines[100][100];  // array of char arrays
  1. Then use strcpy.

Switched:

listOfLines[n] = lineBuffer;
strcpy(listOfLines[n], lineBuffer);

Working:

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

int main(int argc, char *argv[])
{
    FILE *unsortedFile; /* prepare file variable */
    char lineBuffer[100]; /* prepare variable for each line */
    char listOfLines[100][100]; /* prepare line array variable to be sorted */
    int n = 0; 
    int i;
    if (argc == 2) /* if a file has been given */
    {
        unsortedFile = fopen(argv[1], "r"); /* open it readonly */
        if (unsortedFile == NULL) /* if it couldn't open */
        {
            printf("Couldn't open the file %s\n", argv[1]);
            printf("Does it exist?\n");
            return -1; /* stop the program here, return non-zero for error */
        }
        printf("original file:\n\n");
        while (fgets(lineBuffer, sizeof(lineBuffer), unsortedFile))
        {
            printf("%s", lineBuffer);
            strcpy(listOfLines[n], lineBuffer); /* store line buffer to the array */
            n = n + 1; /* increase n for the next array element */
        }
        printf("\nLines to be sorted: %d\n", n);
        for (i = 0; i < n; i++)
        {
            printf("%s", listOfLines[i]);
        }
    } else /* if no or too many args provided */
    {
        printf("\nArgument error - you either didn't supply a filename\n");
        printf("or didn't surround the filename in quotes if it has spaces\n\n");
        return -1; /* return non-zero for error */
    }
}
Tim
  • 2,139
  • 13
  • 18
  • I think you should warn the OP about not use the stack too much. It's could stack overflow :p. By the way return -1 is not valid in main – Stargateur Dec 11 '16 at 16:36
  • I am not very experienced with C, just at troubleshooting. I am respectfully asking for you to explain to me what you mean? Or point me in the direction of some good articles about it? I understand what a StackOverflow is, but I am not experienced enough to see the issue with the code as it is. :) – Tim Dec 11 '16 at 16:38
  • return value of main are ["between 0 and 255"](https://www.gnu.org/software/libc/manual/html_node/Exit-Status.html#Exit-Status). And there link will explain you the stack overflow http://stackoverflow.com/questions/9072415/disadvantages-of-using-large-variables-arrays-on-the-stack http://stackoverflow.com/questions/26158/how-does-a-stack-overflow-occur-and-how-do-you-prevent-it – Stargateur Dec 11 '16 at 16:46
  • Well, this is only about 10k of stack. I've used several megabytes of stack before and it worked fine on Linux. – melpomene Dec 11 '16 at 16:48
  • 1
    @Stargateur Your "between 0 and 255" reference is only for glibc, not C in general (but the only portable exit codes in standard C are 0, `EXIT_SUCESS`, and `EXIT_FAILURE`, which is even more restrictive). – melpomene Dec 11 '16 at 16:49
  • I will change the return codes to something else besides negative one; thanks for letting me know. – giggybyte Dec 11 '16 at 16:52
  • @melpomene If the return type of the main function is a type compatible with int , a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; 11) reaching the } that terminates the main function returns a value of 0. If the return type is not compatible with int, the termination status returned to the host environment is unspecified. http://www.open-std.org/jtc1/SC22/wg14/www/docs/n1548.pdf – Stargateur Dec 11 '16 at 16:56
  • @melpomene So the more portable way is to only use MACRO EXIT_SUCCESS, EXIT_FAILURE or 0, 1. In my opinion of course. – Stargateur Dec 11 '16 at 16:57
  • I read the StackOverflow article linked by Stargateur. Am I to gather that, in C, a pointer puts things on "the heap" and that using that char array puts things on "the stack," i.e., a pointer can't overwrite function code but a char array can? – Tim Dec 11 '16 at 17:00
  • @varlogtim I know that linux have protection to avoid this. Some hacker exploits this before. I can't answer you more. But yes in some case it's possible. But all modern OS avoids this. You need to deactivate manually there protection. – Stargateur Dec 11 '16 at 17:10
  • 1
    @varlogtim It's not about pointers vs. arrays, it's that local variables are generally placed on "the stack" whereas memory allocated with `malloc` comes from "the heap". (Declaring a pointer only creates that pointer; you can make it point to wherever you want.) – melpomene Dec 11 '16 at 18:13