5
\$\begingroup\$

I created my cross-platform implementations of getline function in C.

It takes different arguments and have different return values than 'original' getline function, but aim is the same. The only argument input_file is file from which the line have to be read. Return value is the line read from file or NULL if nothing read.

Here is one implementation, using fgets():

static inline char*
getline(FILE* input_file){
    const unsigned int chunk_size=256;
    char* line=malloc(chunk_size*sizeof*line+1);
    if(line==NULL){
        fprintf(stderr,"Fatal: failed to allocate %zu bytes.\n",chunk_size*sizeof*line+1);
        exit(1);
    }
    unsigned int i;
    for(i=0;;++i){
        memset(line+chunk_size*i,0,chunk_size);
        if(fgets(line+chunk_size*i,chunk_size+1,input_file)==NULL)
            break;
        if(line[strlen(line)-1]=='\n')
            break;
        char* tmp=realloc(line,chunk_size*(i+2)*sizeof*line+1);
        if(tmp==NULL){
            fprintf(stderr,"Fatal: failed to allocate %zu bytes.\n",chunk_size*(i+2)*sizeof*line+1);
            exit(1);
        }else
            line=tmp;
    }
    if(strlen(line)==0){
        free(line);
        return NULL;
    }else{
        line[strlen(line)-1]=0;
        return line;
    }
}

Here is my second implementation, using fgetc():

static inline char*
getline(FILE* input_file){
    const unsigned int chunk_size=3;
    char* line=calloc(chunk_size,sizeof*line+1);
    if(line==NULL){
        fprintf(stderr,"Fatal: failed to allocate %zu bytes.\n",chunk_size*sizeof*line+1);
        exit(1);
    }
    char c;
    unsigned int i,j;
    for(i=0,j=1;;++i){
        c=(char)fgetc(input_file);
        if(c==EOF||c=='\n')
            break;
        line[i]=c;
        if(i==chunk_size*j){
            ++j;
            char* tmp=realloc(line,chunk_size*(j+1)*sizeof*line+1);
            if(tmp==NULL){
                fprintf(stderr,"Fatal: failed to allocate %zu bytes.\n",chunk_size*(j+1)*sizeof*line+1);
                exit(1);
            }else{
                line=tmp;
                memset(line+chunk_size*j,0,chunk_size);
            }
        }
    }
    if(strlen(line)==0){
        free(line);
        return NULL;
    }else{
        line[strlen(line)]=0;
        return line;
    }
}
\$\endgroup\$
2
\$\begingroup\$

exit(1) is probably a bad idea, so is fprintf to stderr. We don't know how big the string can be, therefore running out of memory could be valid, and if stderr is redirected to somewhere a client can see. realloc should already set errno to ENOMEM, so you could probably just return NULL, and modify comment that upon returning NULL, caller should check errno. Or maybe you should return empty string if there's no data, and use NULL to indicate error.

change calloc to malloc, as you anyway NULL terminate the string in your fgetc implementation.

Instead of having this code duplicated for error message (which I vote against), and actual calculation: chunk_size*(j+1)*sizeof*line+1, you could create a variable and use it in both places, therefore you know you print exactly what you did, and there wasn't a mistake if you had to change the calculation slightly.

Try this on windows (as it claims to be a cross platform implementation), but from memory fgetc will return '\r', which you'll put right into the line, whereas I'm pretty sure fgets won't return '\r', when the line terminator is "\r\n"; I believe fgets returns "\n", not "\r\n" on windows, even if "\r\n" is in the input stream.

You don't need to assign data to tmp like this: char* tmp=realloc(line,chunk_size*(i+2)*sizeof*line+1); as upon success you always do: line=tmp, and upon failure, line will no longer point to valid memory. So you could just assign to line.

Notice how you're calling malloc/calloc/realloc, I would add a freeline function to your code in order to free any memory allocated by this code. The caller may not be using the same malloc/calloc/realloc you're using, and their free may not be compatible.

Finally I'm not really sure about this: chunk_size*(j+1)*sizeof*line+1
I think you want: chunk_size*(j+1)*sizeof*char+1, as I think line is of size 4 or maybe even 8?, print it out in the debugger, and see, I think you're allocating a lot more memory then you end up putting into the array.

\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! Your answer has a lot of claims in it ("I'm pretty sure ...", "I think ...") and it would be nice to back these up with some references to documentation resources. \$\endgroup\$ – AlexV Apr 24 at 13:43
  • \$\begingroup\$ I'm pretty sure that although many realloc() implementations set errno as you claim (and POSIX mandates it), that's not in the C Standard, so a conforming implementation may exist which doesn't do that. \$\endgroup\$ – Toby Speight Apr 24 at 14:46
  • \$\begingroup\$ Ok, if the errno is not set for some reason, I would recommend setting it explicitly from the method. This might be a good idea in either case, as it makes the intention explicit, vs. relying on underlying code; Also if there is still going to be an fprintf to report on error, it may change the errno to something else, thus setting it right before returning would be good. \$\endgroup\$ – Alex Apr 24 at 15:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.