17 replies to this topic

[DarkLordoftheMonkeys]

I'm using a linked list for my program. I wrote a function to read lines from a file and store the lines in nodes of the linked list.

Here's the important code:

struct line {
    char *linetext;
    int linenumber;
    struct line *nextline;
};

#define NEXTLINE (*((*lineptr).nextline))

int *numsOfChars( FILE *fp ){
    int numLines = linecount( fp );        
    int *lines = (int *) malloc( sizeof(int) );     // Declare int array.  Must use malloc because
    wchar_t ch;                                     // the address is returned by the function.
    for( int j = 0; j < numLines; j++ ){
        lines[j] = 0;
        while( (ch = fgetc( fp )) != '\n' ){    // For each line n, this goes until the nth 
            lines[j]++;                         // newline, effectively counting the characters
        }                                       // in the nth line until the newline.
    }
    rewind( fp );
    return lines;
}

Most importantly:

// This function reads another line and stores it in the linked list.
// Parameters: lineptr - pointer to the current line, numchars - array of character counts for each line, fp - file pointer
struct line *readLine( struct line *lineptr, int *numchars, FILE *fp ){
    // line number for next line
    int nextLineNum = (*lineptr).linenumber + 1;
    // allocate memory for next line
    (*lineptr).nextline = (struct line *) malloc( sizeof(struct line) );
    // set line number
    NEXTLINE.linenumber = nextLineNum;
    // string to read the line from the file into
    char *linec = (char *) malloc( sizeof(char) * (numchars[nextLineNum] + 1) );
    // read the line of the file into the string
    fgets( linec, (numchars[nextLineNum] + 1), fp );
    // const because strcpy takes a const as its second parameter
    const char *line2c = linec;
    // dynamically allocate NEXTLINE.linetext so it can be copied to
    NEXTLINE.linetext = (char *) malloc( sizeof(char) * (numchars[nextLineNum] + 1) );
    // copy to linetext
    strcpy( NEXTLINE.linetext, line2c );
    free( linec );
    return (*lineptr).nextline;
}

Without comments, if that makes it easier to read:

struct line *readLine( struct line *lineptr, int *numchars, FILE *fp ){
    int nextLineNum = (*lineptr).linenumber + 1;
    (*lineptr).nextline = (struct line *) malloc( sizeof(struct line) );
    NEXTLINE.linenumber = nextLineNum;
    char *linec = (char *) malloc( sizeof(char) * (numchars[nextLineNum] + 1) );
    fgets( linec, (numchars[nextLineNum] + 1), fp );
    const char *line2c = linec;
    NEXTLINE.linetext = (char *) malloc( sizeof(char) * (numchars[nextLineNum] + 1) );
    strcpy( NEXTLINE.linetext, line2c );
    free( linec );
    return (*lineptr).nextline;
}

How I called it:

curline = readLine( curline, numchars, fp );

This works fine for the first few lines, but after I get to about the fifth or sixth one, the linked list gets really huge and the program runs out of memory, resulting in a bus error. Is there any way I can fix this?

[DarkLordoftheMonkeys]

Okay, I fixed the bus error by changing int *lines = (int *) malloc( sizeof(int) ); to int *lines = (int *) malloc( sizeof(int) * numLines ); in that first function. Now I have a newer but less serious problem, which is that the program is counting newlines in places where it shouldn't. I think I need to use wchar_t when reading lines from the file instead of char. Is there a function equivalent to fgets that returns a wchar_t type?

[ZekeDragon]

DarkLordoftheMonkeys said:

I think I need to use wchar_t when reading lines from the file instead of char. Is there a function equivalent to fgets that returns a wchar_t type?

I don't think so, instead you'll need to read the file as a series of bytes and then convert it to wchar_t using a function. This function is mbstowcs().

int main(int argc, char **argv)
{
    // I f****** hate C...
    if (fwide(stdout, 0) == 0)
    {
        if (fwide(stdout, 1) <= 0)
        {
            fprintf(stderr, "Could not switch to wide character mode!\n");
            return 1;
        }
    }

    FILE *f = NULL;
    if (argc == 2)
    {
        if (!(f = fopen(argv[1], "r")))
        {
            fprintf(stderr, "Couldn't open the file for reading!");
            return 1;
        }
    }
    else
    {
        fprintf(stderr, "Incorrect number of arguments passed.");
        return 0;
    }

    char *data = NULL;
    wchar_t *result = NULL;
    size_t val = alloc_read_file(f, &data);
    // alloc_read_file should take two arguments, a FILE * and a char **. The first is the
    // file being read from, the second is just a pointer to a NULL char pointer. It should 
    // read all of the data that is within the file handle, and malloc() the amount of 
    // memory necessary to contain it. alloc_read_file() should then return the number 
    // of bytes read.

    if (data)
    {
        result = malloc(val);
    }

    if (result)
    {
        mbstowcs(result, data, val / sizeof(wchar_t) + 1);
        wprintf("%ls", result);
    }

    free(result);
    free(data);
    return 0;
}

[MeTh0Dz]

There are some serious problems with DarkLordofMonkey's code.

1) You don't ever check if dynamic memory allocation fails. You need to change this.

2) Reading files character by character is extremely inefficient.

3) Your readLine function is silly. You should have two functions. One to read a line, and one to add data to your linked list.

4) Learn to use whitespace in your code. Stuff is hard to read right now.

5) Stop commenting every little step in your code. You really should only be commenting on what functions do and if you do any exotic stuff that may be hard to understand. Using malloc on the otherhand is not hard to understand.

6) Also you've done a really specific linked list implementation. Generally it makes sense to make it more general than just add a container for whatever data types you may be handling. That way the code is more reusable.

And don't start b*tching after you read this as you seem to do.

[DarkLordoftheMonkeys]

I found out what was causing the program to read newlines where it shouldn't have. I did not tell it to read for enough characters, so it was cutting some lines short. I actually did find a function for reading wide characters (fgetws), but I didn't like it so I wrote my own.

Thanks for the help, Zeke. I appreciate it. I'll take a look at the code you gave, even though I seem to have solved the problem.

@Meth0dz: Yes, this code isn't perfect. It's not supposed to be. I am planning to put in error checking later, but I just wanted to get it to work first before I make it more efficient. If you didn't want to read my code with a comment for every line, I did put another identical piece of code, without comments, below it. As for making a specific linked list implementation: like I said, I am not trying to make my code optimal/reusable/self-documenting/anything like that; I am trying to teach myself how to program in C.

[MeTh0Dz]

DarkLordoftheMonkeys said:

I found out what was causing the program to read newlines where it shouldn't have. I did not tell it to read for enough characters, so it was cutting some lines short. I actually did find a function for reading wide characters (fgetws), but I didn't like it so I wrote my own.

Thanks for the help, Zeke. I appreciate it. I'll take a look at the code you gave, even though I seem to have solved the problem.

@Meth0dz: Yes, this code isn't perfect. It's not supposed to be. I am planning to put in error checking later, but I just wanted to get it to work first before I make it more efficient. If you didn't want to read my code with a comment for every line, I did put another identical piece of code, without comments, below it. As for making a specific linked list implementation: like I said, I am not trying to make my code optimal/reusable/self-documenting/anything like that; I am trying to teach myself how to program in C.

If you want to learn how to program in C, you should learn how to do it properly, which I can help you do.

[DarkLordoftheMonkeys]

MeTh0Dz said:

If you want to learn how to program in C, you should learn how to do it properly, which I can help you do.

I need to figure out the basics, just how to do it, before I learn how to do it well. Programming style and code optimization are things that can come after I've grasped what everything in C actually does. I don't think you're trying to help me. I think you just like to bicker, that's all. You're a flamer.

[MeTh0Dz]

DarkLordoftheMonkeys said:

I need to figure out the basics, just how to do it, before I learn how to do it well. Programming style and code optimization are things that can come after I've grasped what everything in C actually does. I don't think you're trying to help me. I think you just like to bicker, that's all. You're a flamer.

You just sound like a butt hurt kid. Correcting bad code is not flaming. If you post code, expect me to correct it.

[DarkLordoftheMonkeys]

MeTh0Dz said:

You just sound like a butt hurt kid. Correcting bad code is not flaming. If you post code, expect me to correct it.

That would be easier to believe if I hadn't seen several other threads of you carrying on shouting matches with other members. I don't know how I came off as butthurt. If I was I would be cussing or USING ALL CAPS or something like that. :P

[MeTh0Dz]

DarkLordoftheMonkeys said:

That would be easier to believe if I hadn't seen several other threads of you carrying on shouting matches with other members. I don't know how I came off as butthurt. If I was I would be cussing or USING ALL CAPS or something like that. :P

Find a thread within the past 3 months where I was involved in a shouting match.

Also my ability to correct crap C code has nothing to do with how much I 'shout'. All of my suggestions are legitimate. And as someone who admits to have poor C knowledge you are in no real position to contest things that I say.

[DarkLordoftheMonkeys]

Here's one.
Yes, it's not literally shouting, but you like to argue with people for the sake of arguing. So no, I don't think you're trying to help me. If you were, you would have offered suggestions for how I could fix the code, rather than merely stating what was wrong with it and leaving me in the dark. None of what you did suggest would have solved the problems I was having, so if you were trying to help me, you failed.

[MeTh0Dz]

DarkLordoftheMonkeys said:

Here's one.
Yes, it's not literally shouting, but you like to argue with people for the sake of arguing. So no, I don't think you're trying to help me. If you were, you would have offered suggestions for how I could fix the code, rather than merely stating what was wrong with it and leaving me in the dark. None of what you did suggest would have solved the problems I was having, so if you were trying to help me, you failed.

I only argue when people are wrong and won't let it go. If you'd like to point out somewhere where I was wrong and continued to argue I'd like to get a look.

If you can't figure out how to fix it all you need to do is ask. I assumed you'd be resourceful enough to figure it out but apparently not.