Dynamic fgets in C

.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;

up vote
6
down vote

favorite

I wanted to make a function that will dynamically retrieve a line from a stream to a buffer. This function just needs to take in the char* and the stream to read from. It'll keep allocating a bigger buffer until it can read all the data into the rtr variable (terminated on new line). Any ways I can improve this would be amazing.

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

char* dynamic_fgets(char** rtr, FILE* stream) 
 int bufsize = 1024; //Start at 1024 bytes
 char* buf = (char*) malloc(bufsize*sizeof(char));

 if(buf == NULL) 
 perror("Couldn't allocate memory for buf in dynamic_fgetsn");
 

 do 
 fgets(buf, bufsize, stream);
 *rtr = realloc(*rtr, strlen(buf)+strlen(*rtr));
 if(*rtr == NULL) 
 perror("Couldn't allocate memory for *rtr in dynamic_fgetsn");
 

 *rtr = strncat(*rtr, buf, bufsize);
 bufsize *= 2;
 while(buf[strlen(buf)-1] != 'n');

 return *rtr;


int main(int argc, char** argv) 
 char* buf = (char*) malloc(sizeof(1));
 strncpy(buf, "", 1);
 printf("Input: ");

 dynamic_fgets(&buf, stdin);

 printf("Output: %s", buf);

 return 0;

edited Aug 26 at 18:49

Daniel

4,1232836

asked Aug 26 at 1:09

Axel Persinger

312

Perhaps you want getline(3) ?
â€“Â Basile Starynkevitch
Aug 26 at 22:57

add a commentÂ |Â

up vote
6
down vote

favorite

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

char* dynamic_fgets(char** rtr, FILE* stream) 
 int bufsize = 1024; //Start at 1024 bytes
 char* buf = (char*) malloc(bufsize*sizeof(char));

 if(buf == NULL) 
 perror("Couldn't allocate memory for buf in dynamic_fgetsn");
 

 do 
 fgets(buf, bufsize, stream);
 *rtr = realloc(*rtr, strlen(buf)+strlen(*rtr));
 if(*rtr == NULL) 
 perror("Couldn't allocate memory for *rtr in dynamic_fgetsn");
 

 *rtr = strncat(*rtr, buf, bufsize);
 bufsize *= 2;
 while(buf[strlen(buf)-1] != 'n');

 return *rtr;


int main(int argc, char** argv) 
 char* buf = (char*) malloc(sizeof(1));
 strncpy(buf, "", 1);
 printf("Input: ");

 dynamic_fgets(&buf, stdin);

 printf("Output: %s", buf);

 return 0;

edited Aug 26 at 18:49

Daniel

4,1232836

asked Aug 26 at 1:09

Axel Persinger

312

Perhaps you want getline(3) ?
â€“Â Basile Starynkevitch
Aug 26 at 22:57

add a commentÂ |Â

up vote
6
down vote

favorite

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

char* dynamic_fgets(char** rtr, FILE* stream) 
 int bufsize = 1024; //Start at 1024 bytes
 char* buf = (char*) malloc(bufsize*sizeof(char));

 if(buf == NULL) 
 perror("Couldn't allocate memory for buf in dynamic_fgetsn");
 

 do 
 fgets(buf, bufsize, stream);
 *rtr = realloc(*rtr, strlen(buf)+strlen(*rtr));
 if(*rtr == NULL) 
 perror("Couldn't allocate memory for *rtr in dynamic_fgetsn");
 

 *rtr = strncat(*rtr, buf, bufsize);
 bufsize *= 2;
 while(buf[strlen(buf)-1] != 'n');

 return *rtr;


int main(int argc, char** argv) 
 char* buf = (char*) malloc(sizeof(1));
 strncpy(buf, "", 1);
 printf("Input: ");

 dynamic_fgets(&buf, stdin);

 printf("Output: %s", buf);

 return 0;

edited Aug 26 at 18:49

Daniel

4,1232836

asked Aug 26 at 1:09

Axel Persinger

312

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

char* dynamic_fgets(char** rtr, FILE* stream) 
 int bufsize = 1024; //Start at 1024 bytes
 char* buf = (char*) malloc(bufsize*sizeof(char));

 if(buf == NULL) 
 perror("Couldn't allocate memory for buf in dynamic_fgetsn");
 

 do 
 fgets(buf, bufsize, stream);
 *rtr = realloc(*rtr, strlen(buf)+strlen(*rtr));
 if(*rtr == NULL) 
 perror("Couldn't allocate memory for *rtr in dynamic_fgetsn");
 

 *rtr = strncat(*rtr, buf, bufsize);
 bufsize *= 2;
 while(buf[strlen(buf)-1] != 'n');

 return *rtr;


int main(int argc, char** argv) 
 char* buf = (char*) malloc(sizeof(1));
 strncpy(buf, "", 1);
 printf("Input: ");

 dynamic_fgets(&buf, stdin);

 printf("Output: %s", buf);

 return 0;

edited Aug 26 at 18:49

Daniel

4,1232836

asked Aug 26 at 1:09

Axel Persinger

312

edited Aug 26 at 18:49

Daniel

4,1232836

edited Aug 26 at 18:49

Daniel

4,1232836

edited Aug 26 at 18:49

Daniel

4,1232836

asked Aug 26 at 1:09

Axel Persinger

312

asked Aug 26 at 1:09

Axel Persinger

312

asked Aug 26 at 1:09

Axel Persinger

312

Perhaps you want getline(3) ?
â€“Â Basile Starynkevitch
Aug 26 at 22:57

add a commentÂ |Â

Perhaps you want getline(3) ?
â€“Â Basile Starynkevitch
Aug 26 at 22:57

Perhaps you want getline(3) ?
â€“Â Basile Starynkevitch
Aug 26 at 22:57

add a commentÂ |Â

5 Answers
5

active

oldest

votes

up vote
10
down vote

Code runs into undefined behavior (UB)

Code fails to test the return value of fgets(). Without knowing the return value is not NULL, strlen(buf) can get called on 1) uninitialized memory or 2) memory in an undefined state or 3) buf from the previous fgets() call.

// UB possible
fgets(buf, bufsize, stream);
*rtr = realloc(*rtr, strlen(buf)+strlen(*rtr));

// Better
if (fgets(buf, bufsize, stream)) {
 ... strlen(buf) ...

Infinite loop

Should user input end with a non-'n' (think piped input), do loop does not stop before UB kicks in.

Exploitable code

dynamic_fgets(&buf, stdin); allows an external user make the program use excessive resources. There is no upper bound. Defensive coding does not allow memory consumption to go unchecked from potential hostile users or pathological file input.

IMO, dynamic_fgets() , deserves an upper bound parameter. Once exceeded, the routine should not allocative more memory and instead return an error indication.

Code is fooled with ''

How robust do you want your code?

In hostile cases, fgets() may read a null character and fgets() treats it no different than any other non 'n' character. A following strlen(buf) can stop on the usual null character appended by fgets(), yet it will first stop on a read null character. strlen(buf) could be 0 due to such input.

Further this code is then readily exploitable due to buf[strlen(buf)-1] by entering a null character as the first input character. The code is the same as buf[SIZE_MAX] an access outside the allocation and thus UB. At a minimum, do not code buf[strlen(buf)-1] when buf[0] == 0 is possible.

size_t vs. int

Minor: size_t is the right width for array sizing. It is the type return by strlen() and accepted by *alloc(). int can be insufficient for long strings.

edited Aug 26 at 8:38

answered Aug 26 at 3:54

chux

11.5k11239

2

Code is fooled with '' is an interesting exploit indeed.
â€“Â vnp
Aug 26 at 8:04

add a commentÂ |Â

up vote
8
down vote

Make up your mind. dynamic_fgets returns the same information via return value (return *rtr) and the in-out parameter (char ** rtr). Chose one.

Do not cast malloc return value. In C it is redundant, and may cause hard-to-find bugs.

If, for any reason, you feel you need to cast it, be consistent and cast realloc as well.

Ditto for sizeof(char). It is 1 by definition; if you still want to spell it out in a call to malloc, do so for realloc.

*rtr = realloc(*rtr, ....) is dangerous, for it leads to memory leak. Shall realloc fail, the memory pointed by *rtr prior to the call is lost. Consider instead

char * tmp = realloc(*rtr, ....);
if (tmp == NULL) 
 // whatever recovery strategy you may come up with, e.g. free(*rtr);
 ....
 return error_indication;

*rtr = tmp;
.... // Business as usual

Every call to dynamic_fgets allocates 1024 bytes for buf, which is never freed. 1024 bytes per call leaked.

An unconditional reallocation looks strange. Chances are that what fgets have read is fitting into an allocated space just fine. Consider testing for n first, and realloc only if necessary.

strncat(*rtr, buf, bufsize) must find a terminating in a memory pointed by *rtr, which leads to quadratic time complexity. You should track where the terminator was placed by the previous iteration.

buf is unnecessary. You may read directly into *rtr (plus offset, see the above bullet), thus sparing a copy.

answered Aug 26 at 2:33

vnp

36.8k12992

add a commentÂ |Â

up vote
5
down vote

I suggest using the POSIX getline() function as it does what the posted code is trying to do. However, it does take a third parameter (a pointer to a variable to receive the length of the read line). It also returns the number of bytes read (or an EOF indicator).

edited Aug 26 at 3:29

200_success

124k14144401

answered Aug 26 at 2:49

user3629249

1,55249

1

Thanks, I appreciate the suggestion, but I wanted to make something myself so I could understand the language better
â€“Â Axel Persinger
Aug 26 at 12:51

1

@AxelPersinger Even if you want to implement something from scratch, it's still a good idea to learn from the design of existing solutions. In particular, the second parameter and the return value differ from yours, and you should consider why.
â€“Â 200_success
Aug 26 at 14:51

add a commentÂ |Â

up vote
4
down vote

Bug: buffer overflow

You only ever allocate buf once, with size 1024, here:

int bufsize = 1024; //Start at 1024 bytes
char* buf = (char*) malloc(bufsize*sizeof(char));

But later, you double bufsize on every iteration through the loop, without also doubling the allocation of buf.

Therefore, on the second iteration of the loop, when you run this line:

 fgets(buf, bufsize, stream);

you will overflow your buffer, because buf has 1024 bytes, but bufsize will be 2048. As user vnp mentioned, you don't really even need the buf variable if you just read to the end of *rtr and keep track of the number of bytes read so far.

Shlemiel the painter

There is a funny story about a painter who paints slower and slower every day. The punchline:

"I can't help it," says Shlemiel. "Every day I get farther and farther
away from the paint can!"

This story is actually about concatenating strings. In your loop, there are the following operations:

strlen(buf) x2
strlen(*rtr)
strncat(*rtr)

You can't avoid one call to strlen(buf) but the calls involving rtr can be avoided by simply tracking the length of rtr in some variable.

answered Aug 26 at 23:05

JS1

27k32876

add a commentÂ |Â

up vote
3
down vote

One more thing not mentioned yet. Dont put command line arguments if you dont use them in the code. It is just misleading since you dont use argc and argv. consider to use simply int main (void)

edited Aug 27 at 9:42

answered Aug 26 at 21:02

Sandro4912

637120

Shouldn't that be int main(void)?
â€“Â Thomas Padron-McCarthy
Aug 27 at 8:08

you are right in c++ its without the void but not in c. i will edit it. see google.de/amp/s/www.geeksforgeeks.org/â€¦
â€“Â Sandro4912
Aug 27 at 9:42

add a commentÂ |Â

Your Answer

StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");

StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: false,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);

);

draft saved

draft discarded

StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f202490%2fdynamic-fgets-in-c%23new-answer', 'question_page');

);

Post as a guest

Name

5 Answers
5

active

oldest

votes

5 Answers
5

active

oldest

votes

up vote
10
down vote

Code runs into undefined behavior (UB)

// UB possible
fgets(buf, bufsize, stream);
*rtr = realloc(*rtr, strlen(buf)+strlen(*rtr));

// Better
if (fgets(buf, bufsize, stream)) {
 ... strlen(buf) ...

Infinite loop

Should user input end with a non-'n' (think piped input), do loop does not stop before UB kicks in.

Exploitable code

IMO, dynamic_fgets() , deserves an upper bound parameter. Once exceeded, the routine should not allocative more memory and instead return an error indication.

Code is fooled with ''

How robust do you want your code?

size_t vs. int

Minor: size_t is the right width for array sizing. It is the type return by strlen() and accepted by *alloc(). int can be insufficient for long strings.

edited Aug 26 at 8:38

answered Aug 26 at 3:54

chux

11.5k11239

2

Code is fooled with '' is an interesting exploit indeed.
â€“Â vnp
Aug 26 at 8:04

add a commentÂ |Â

up vote
10
down vote

Code runs into undefined behavior (UB)

// UB possible
fgets(buf, bufsize, stream);
*rtr = realloc(*rtr, strlen(buf)+strlen(*rtr));

// Better
if (fgets(buf, bufsize, stream)) {
 ... strlen(buf) ...

Infinite loop

Should user input end with a non-'n' (think piped input), do loop does not stop before UB kicks in.

Exploitable code

IMO, dynamic_fgets() , deserves an upper bound parameter. Once exceeded, the routine should not allocative more memory and instead return an error indication.

Code is fooled with ''

How robust do you want your code?

size_t vs. int

Minor: size_t is the right width for array sizing. It is the type return by strlen() and accepted by *alloc(). int can be insufficient for long strings.

edited Aug 26 at 8:38

answered Aug 26 at 3:54

chux

11.5k11239

2

Code is fooled with '' is an interesting exploit indeed.
â€“Â vnp
Aug 26 at 8:04

add a commentÂ |Â

up vote
10
down vote

Code runs into undefined behavior (UB)

// UB possible
fgets(buf, bufsize, stream);
*rtr = realloc(*rtr, strlen(buf)+strlen(*rtr));

// Better
if (fgets(buf, bufsize, stream)) {
 ... strlen(buf) ...

Infinite loop

Should user input end with a non-'n' (think piped input), do loop does not stop before UB kicks in.

Exploitable code

IMO, dynamic_fgets() , deserves an upper bound parameter. Once exceeded, the routine should not allocative more memory and instead return an error indication.

Code is fooled with ''

How robust do you want your code?

size_t vs. int

Minor: size_t is the right width for array sizing. It is the type return by strlen() and accepted by *alloc(). int can be insufficient for long strings.

edited Aug 26 at 8:38

answered Aug 26 at 3:54

chux

11.5k11239

Code runs into undefined behavior (UB)

// UB possible
fgets(buf, bufsize, stream);
*rtr = realloc(*rtr, strlen(buf)+strlen(*rtr));

// Better
if (fgets(buf, bufsize, stream)) {
 ... strlen(buf) ...

Infinite loop

Should user input end with a non-'n' (think piped input), do loop does not stop before UB kicks in.

Exploitable code

IMO, dynamic_fgets() , deserves an upper bound parameter. Once exceeded, the routine should not allocative more memory and instead return an error indication.

Code is fooled with ''

How robust do you want your code?

size_t vs. int

Minor: size_t is the right width for array sizing. It is the type return by strlen() and accepted by *alloc(). int can be insufficient for long strings.

edited Aug 26 at 8:38

answered Aug 26 at 3:54

chux

11.5k11239

edited Aug 26 at 8:38

answered Aug 26 at 3:54

chux

11.5k11239

answered Aug 26 at 3:54

chux

11.5k11239

answered Aug 26 at 3:54

chux

11.5k11239

2

Code is fooled with '' is an interesting exploit indeed.
â€“Â vnp
Aug 26 at 8:04

add a commentÂ |Â

2

Code is fooled with '' is an interesting exploit indeed.
â€“Â vnp
Aug 26 at 8:04

Code is fooled with '' is an interesting exploit indeed.
â€“Â vnp
Aug 26 at 8:04

add a commentÂ |Â

up vote
8
down vote

Make up your mind. dynamic_fgets returns the same information via return value (return *rtr) and the in-out parameter (char ** rtr). Chose one.

Do not cast malloc return value. In C it is redundant, and may cause hard-to-find bugs.

If, for any reason, you feel you need to cast it, be consistent and cast realloc as well.

Ditto for sizeof(char). It is 1 by definition; if you still want to spell it out in a call to malloc, do so for realloc.

*rtr = realloc(*rtr, ....) is dangerous, for it leads to memory leak. Shall realloc fail, the memory pointed by *rtr prior to the call is lost. Consider instead

char * tmp = realloc(*rtr, ....);
if (tmp == NULL) 
 // whatever recovery strategy you may come up with, e.g. free(*rtr);
 ....
 return error_indication;

*rtr = tmp;
.... // Business as usual

Every call to dynamic_fgets allocates 1024 bytes for buf, which is never freed. 1024 bytes per call leaked.

An unconditional reallocation looks strange. Chances are that what fgets have read is fitting into an allocated space just fine. Consider testing for n first, and realloc only if necessary.

strncat(*rtr, buf, bufsize) must find a terminating in a memory pointed by *rtr, which leads to quadratic time complexity. You should track where the terminator was placed by the previous iteration.

buf is unnecessary. You may read directly into *rtr (plus offset, see the above bullet), thus sparing a copy.

answered Aug 26 at 2:33

vnp

36.8k12992

add a commentÂ |Â

up vote
8
down vote

Make up your mind. dynamic_fgets returns the same information via return value (return *rtr) and the in-out parameter (char ** rtr). Chose one.

Do not cast malloc return value. In C it is redundant, and may cause hard-to-find bugs.

If, for any reason, you feel you need to cast it, be consistent and cast realloc as well.

Ditto for sizeof(char). It is 1 by definition; if you still want to spell it out in a call to malloc, do so for realloc.

*rtr = realloc(*rtr, ....) is dangerous, for it leads to memory leak. Shall realloc fail, the memory pointed by *rtr prior to the call is lost. Consider instead

char * tmp = realloc(*rtr, ....);
if (tmp == NULL) 
 // whatever recovery strategy you may come up with, e.g. free(*rtr);
 ....
 return error_indication;

*rtr = tmp;
.... // Business as usual

Every call to dynamic_fgets allocates 1024 bytes for buf, which is never freed. 1024 bytes per call leaked.

An unconditional reallocation looks strange. Chances are that what fgets have read is fitting into an allocated space just fine. Consider testing for n first, and realloc only if necessary.

strncat(*rtr, buf, bufsize) must find a terminating in a memory pointed by *rtr, which leads to quadratic time complexity. You should track where the terminator was placed by the previous iteration.

buf is unnecessary. You may read directly into *rtr (plus offset, see the above bullet), thus sparing a copy.

answered Aug 26 at 2:33

vnp

36.8k12992

add a commentÂ |Â

up vote
8
down vote

Make up your mind. dynamic_fgets returns the same information via return value (return *rtr) and the in-out parameter (char ** rtr). Chose one.

Do not cast malloc return value. In C it is redundant, and may cause hard-to-find bugs.

If, for any reason, you feel you need to cast it, be consistent and cast realloc as well.

Ditto for sizeof(char). It is 1 by definition; if you still want to spell it out in a call to malloc, do so for realloc.

*rtr = realloc(*rtr, ....) is dangerous, for it leads to memory leak. Shall realloc fail, the memory pointed by *rtr prior to the call is lost. Consider instead

char * tmp = realloc(*rtr, ....);
if (tmp == NULL) 
 // whatever recovery strategy you may come up with, e.g. free(*rtr);
 ....
 return error_indication;

*rtr = tmp;
.... // Business as usual

Every call to dynamic_fgets allocates 1024 bytes for buf, which is never freed. 1024 bytes per call leaked.

An unconditional reallocation looks strange. Chances are that what fgets have read is fitting into an allocated space just fine. Consider testing for n first, and realloc only if necessary.

strncat(*rtr, buf, bufsize) must find a terminating in a memory pointed by *rtr, which leads to quadratic time complexity. You should track where the terminator was placed by the previous iteration.

buf is unnecessary. You may read directly into *rtr (plus offset, see the above bullet), thus sparing a copy.

answered Aug 26 at 2:33

vnp

36.8k12992

Make up your mind. dynamic_fgets returns the same information via return value (return *rtr) and the in-out parameter (char ** rtr). Chose one.

Do not cast malloc return value. In C it is redundant, and may cause hard-to-find bugs.

If, for any reason, you feel you need to cast it, be consistent and cast realloc as well.

Ditto for sizeof(char). It is 1 by definition; if you still want to spell it out in a call to malloc, do so for realloc.

*rtr = realloc(*rtr, ....) is dangerous, for it leads to memory leak. Shall realloc fail, the memory pointed by *rtr prior to the call is lost. Consider instead

char * tmp = realloc(*rtr, ....);
if (tmp == NULL) 
 // whatever recovery strategy you may come up with, e.g. free(*rtr);
 ....
 return error_indication;

*rtr = tmp;
.... // Business as usual

Every call to dynamic_fgets allocates 1024 bytes for buf, which is never freed. 1024 bytes per call leaked.

An unconditional reallocation looks strange. Chances are that what fgets have read is fitting into an allocated space just fine. Consider testing for n first, and realloc only if necessary.

strncat(*rtr, buf, bufsize) must find a terminating in a memory pointed by *rtr, which leads to quadratic time complexity. You should track where the terminator was placed by the previous iteration.

buf is unnecessary. You may read directly into *rtr (plus offset, see the above bullet), thus sparing a copy.

answered Aug 26 at 2:33

vnp

36.8k12992

answered Aug 26 at 2:33

vnp

36.8k12992

answered Aug 26 at 2:33

vnp

36.8k12992

answered Aug 26 at 2:33

vnp

36.8k12992

add a commentÂ |Â

up vote
5
down vote

edited Aug 26 at 3:29

200_success

124k14144401

answered Aug 26 at 2:49

user3629249

1,55249

1

Thanks, I appreciate the suggestion, but I wanted to make something myself so I could understand the language better
â€“Â Axel Persinger
Aug 26 at 12:51

1

@AxelPersinger Even if you want to implement something from scratch, it's still a good idea to learn from the design of existing solutions. In particular, the second parameter and the return value differ from yours, and you should consider why.
â€“Â 200_success
Aug 26 at 14:51

add a commentÂ |Â

up vote
5
down vote

edited Aug 26 at 3:29

200_success

124k14144401

answered Aug 26 at 2:49

user3629249

1,55249

1

Thanks, I appreciate the suggestion, but I wanted to make something myself so I could understand the language better
â€“Â Axel Persinger
Aug 26 at 12:51

1

@AxelPersinger Even if you want to implement something from scratch, it's still a good idea to learn from the design of existing solutions. In particular, the second parameter and the return value differ from yours, and you should consider why.
â€“Â 200_success
Aug 26 at 14:51

add a commentÂ |Â

up vote
5
down vote

edited Aug 26 at 3:29

200_success

124k14144401

answered Aug 26 at 2:49

user3629249

1,55249

edited Aug 26 at 3:29

200_success

124k14144401

answered Aug 26 at 2:49

user3629249

1,55249

edited Aug 26 at 3:29

200_success

124k14144401

edited Aug 26 at 3:29

200_success

124k14144401

edited Aug 26 at 3:29

200_success

124k14144401

answered Aug 26 at 2:49

user3629249

1,55249

answered Aug 26 at 2:49

user3629249

1,55249

answered Aug 26 at 2:49

user3629249

1,55249

1

Thanks, I appreciate the suggestion, but I wanted to make something myself so I could understand the language better
â€“Â Axel Persinger
Aug 26 at 12:51

1

@AxelPersinger Even if you want to implement something from scratch, it's still a good idea to learn from the design of existing solutions. In particular, the second parameter and the return value differ from yours, and you should consider why.
â€“Â 200_success
Aug 26 at 14:51

add a commentÂ |Â

1

Thanks, I appreciate the suggestion, but I wanted to make something myself so I could understand the language better
â€“Â Axel Persinger
Aug 26 at 12:51

1

@AxelPersinger Even if you want to implement something from scratch, it's still a good idea to learn from the design of existing solutions. In particular, the second parameter and the return value differ from yours, and you should consider why.
â€“Â 200_success
Aug 26 at 14:51

Thanks, I appreciate the suggestion, but I wanted to make something myself so I could understand the language better
â€“Â Axel Persinger
Aug 26 at 12:51

@AxelPersinger Even if you want to implement something from scratch, it's still a good idea to learn from the design of existing solutions. In particular, the second parameter and the return value differ from yours, and you should consider why.
â€“Â 200_success
Aug 26 at 14:51

add a commentÂ |Â

up vote
4
down vote

Bug: buffer overflow

You only ever allocate buf once, with size 1024, here:

int bufsize = 1024; //Start at 1024 bytes
char* buf = (char*) malloc(bufsize*sizeof(char));

But later, you double bufsize on every iteration through the loop, without also doubling the allocation of buf.

Therefore, on the second iteration of the loop, when you run this line:

 fgets(buf, bufsize, stream);

Shlemiel the painter

There is a funny story about a painter who paints slower and slower every day. The punchline:

"I can't help it," says Shlemiel. "Every day I get farther and farther
away from the paint can!"

This story is actually about concatenating strings. In your loop, there are the following operations:

strlen(buf) x2
strlen(*rtr)
strncat(*rtr)

You can't avoid one call to strlen(buf) but the calls involving rtr can be avoided by simply tracking the length of rtr in some variable.

answered Aug 26 at 23:05

JS1

27k32876

add a commentÂ |Â

up vote
4
down vote

Bug: buffer overflow

You only ever allocate buf once, with size 1024, here:

int bufsize = 1024; //Start at 1024 bytes
char* buf = (char*) malloc(bufsize*sizeof(char));

But later, you double bufsize on every iteration through the loop, without also doubling the allocation of buf.

Therefore, on the second iteration of the loop, when you run this line:

 fgets(buf, bufsize, stream);

Shlemiel the painter

There is a funny story about a painter who paints slower and slower every day. The punchline:

"I can't help it," says Shlemiel. "Every day I get farther and farther
away from the paint can!"

This story is actually about concatenating strings. In your loop, there are the following operations:

strlen(buf) x2
strlen(*rtr)
strncat(*rtr)

You can't avoid one call to strlen(buf) but the calls involving rtr can be avoided by simply tracking the length of rtr in some variable.

answered Aug 26 at 23:05

JS1

27k32876

add a commentÂ |Â

up vote
4
down vote

Bug: buffer overflow

You only ever allocate buf once, with size 1024, here:

int bufsize = 1024; //Start at 1024 bytes
char* buf = (char*) malloc(bufsize*sizeof(char));

But later, you double bufsize on every iteration through the loop, without also doubling the allocation of buf.

Therefore, on the second iteration of the loop, when you run this line:

 fgets(buf, bufsize, stream);

Shlemiel the painter

There is a funny story about a painter who paints slower and slower every day. The punchline:

"I can't help it," says Shlemiel. "Every day I get farther and farther
away from the paint can!"

This story is actually about concatenating strings. In your loop, there are the following operations:

strlen(buf) x2
strlen(*rtr)
strncat(*rtr)

You can't avoid one call to strlen(buf) but the calls involving rtr can be avoided by simply tracking the length of rtr in some variable.

answered Aug 26 at 23:05

JS1

27k32876

Bug: buffer overflow

You only ever allocate buf once, with size 1024, here:

int bufsize = 1024; //Start at 1024 bytes
char* buf = (char*) malloc(bufsize*sizeof(char));

But later, you double bufsize on every iteration through the loop, without also doubling the allocation of buf.

Therefore, on the second iteration of the loop, when you run this line:

 fgets(buf, bufsize, stream);

Shlemiel the painter

There is a funny story about a painter who paints slower and slower every day. The punchline:

"I can't help it," says Shlemiel. "Every day I get farther and farther
away from the paint can!"

This story is actually about concatenating strings. In your loop, there are the following operations:

strlen(buf) x2
strlen(*rtr)
strncat(*rtr)

You can't avoid one call to strlen(buf) but the calls involving rtr can be avoided by simply tracking the length of rtr in some variable.

answered Aug 26 at 23:05

JS1

27k32876

answered Aug 26 at 23:05

JS1

27k32876

answered Aug 26 at 23:05

JS1

27k32876

answered Aug 26 at 23:05

JS1

27k32876

add a commentÂ |Â

up vote
3
down vote

edited Aug 27 at 9:42

answered Aug 26 at 21:02

Sandro4912

637120

Shouldn't that be int main(void)?
â€“Â Thomas Padron-McCarthy
Aug 27 at 8:08

you are right in c++ its without the void but not in c. i will edit it. see google.de/amp/s/www.geeksforgeeks.org/â€¦
â€“Â Sandro4912
Aug 27 at 9:42

add a commentÂ |Â

up vote
3
down vote

edited Aug 27 at 9:42

answered Aug 26 at 21:02

Sandro4912

637120

Shouldn't that be int main(void)?
â€“Â Thomas Padron-McCarthy
Aug 27 at 8:08

you are right in c++ its without the void but not in c. i will edit it. see google.de/amp/s/www.geeksforgeeks.org/â€¦
â€“Â Sandro4912
Aug 27 at 9:42

add a commentÂ |Â

up vote
3
down vote

edited Aug 27 at 9:42

answered Aug 26 at 21:02

Sandro4912

637120

edited Aug 27 at 9:42

answered Aug 26 at 21:02

Sandro4912

637120

edited Aug 27 at 9:42

answered Aug 26 at 21:02

Sandro4912

637120

answered Aug 26 at 21:02

Sandro4912

637120

answered Aug 26 at 21:02

Sandro4912

637120

Shouldn't that be int main(void)?
â€“Â Thomas Padron-McCarthy
Aug 27 at 8:08

you are right in c++ its without the void but not in c. i will edit it. see google.de/amp/s/www.geeksforgeeks.org/â€¦
â€“Â Sandro4912
Aug 27 at 9:42

add a commentÂ |Â

Shouldn't that be int main(void)?
â€“Â Thomas Padron-McCarthy
Aug 27 at 8:08

you are right in c++ its without the void but not in c. i will edit it. see google.de/amp/s/www.geeksforgeeks.org/â€¦
â€“Â Sandro4912
Aug 27 at 9:42

Shouldn't that be int main(void)?
â€“Â Thomas Padron-McCarthy
Aug 27 at 8:08

you are right in c++ its without the void but not in c. i will edit it. see google.de/amp/s/www.geeksforgeeks.org/â€¦
â€“Â Sandro4912
Aug 27 at 9:42

add a commentÂ |Â

draft saved

draft discarded

draft saved

draft discarded

Post as a guest

Name

Search This Blog

Iyfjky