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;
}
}
1 Answer 1
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.
-
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– AlexV2019年04月24日 13:43:57 +00:00Commented Apr 24, 2019 at 13:43
-
\$\begingroup\$ I'm pretty sure that although many
realloc()
implementations seterrno
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– Toby Speight2019年04月24日 14:46:42 +00:00Commented Apr 24, 2019 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– Alex2019年04月24日 15:05:40 +00:00Commented Apr 24, 2019 at 15:05
Explore related questions
See similar questions with these tags.