Using fgets() in a proper way

Question

We have a project for this semester in embedded systems programming (C). I made a client/server program(Chat program) but my professor said when i am using the fgets it is not resource friendly but i don't know why.

Here is my source:

if (argc == 4){
   strcpy(nick, argv[3]);
   strcat(nick,":");
}
else if (argc == 3){
    printf ("Please type your Nickname!\n");
    fgets (nick,sizeof(nick),stdin);
    nick[strlen(nick)-1]=':';
}
else{
    usage (argv[0]);
}

I don't see anything wrong (or resource unfriendly) in your snippet. I'd (maybe) just save the string length in a variable for later use. — pmg, May 28 '15 at 14:41
@pmg maybe not exactly related but a pointer `nick` will cause issue in `sizeof(nick)`, isn't it? — Sourav Ghosh, May 28 '15 at 14:44
Yes, `sizeof ` is wrong ... but that is not a resource friendliness issue: it's a plain programming error. — pmg, May 28 '15 at 14:45
@pmg Agree 100%, but then, you never know, what is a _resource_ here. :-) — Sourav Ghosh, May 28 '15 at 14:47
The professor *migh* mean that usually one uses a fixed-size buffer when calling `fgets`, and if the input is small then maybe a lot of the buffer-space is wasted. On small embedded systems that could mean a lot. — Some programmer dude, May 28 '15 at 14:47
The code works properly but my professor said what if i give a too big nickname. (1.000.000 characters) — Gabesz, May 28 '15 at 14:47
You need to nail down a requirement or two: what does your professor want you to do with a long (1000000 characters) nickname? Truncate it? Return an error? Allocate 1MB of memory for it? — Michael Burr, May 28 '15 at 20:21
Really this is a question for your professor; did you think to ask him whet he meant? That is his job after all. The `fgets()` call will only ever read `sizeof(nick)` characters, so it is still unclear what your professor's concern is without greater context (though there are plenty of other concerns). An "embedded system" that supports a process loader (implied by argv/argc) is atypical of most embedded systems and probably not particularly resource constrained. I see nothing in this that teaches anything useful or specific about embedded systems. — Clifford, May 31 '15 at 08:19
I'd be more concerned about the `strcpy()` and `strcat()` calls - there you have no check that `nick` is large enough to take the `argv[3]` string and its '`:`' suffix. — Clifford, May 31 '15 at 08:30

score 1 · Answer 1 · answered May 28 '15 at 15:05

1

The solution is then of course to "manually" read the input, resulting in lots of instructions in C but being carefull with resources and CPU:

else if (argc == 3) {
    printf ("Please type your Nickname!\n");
    int i= 0;
    while ((c=fgetc(stdin))!='\n' && i<sizeof(nick)-2) nick[i++]= c;
    nick[i++]=':'; nick[i++]= '\0';
}

(This assumes nick is an array of characters, not a pointer. Otherwise the size of nick must be given as a parameter and sizeof(nick)-2 becomes nick_size-2.)

answered May 28 '15 at 15:05

Paul Ogilvie

25,048
4
23
41

Thanks for your answers! – Gabesz May 28 '15 at 15:13
I cannot see what has been achieved here in terms of "resource". `fgets()` itself probably uses few resources - you have added an variable `c` and `nick` remains the same as it was (and we have no idea what that is from the code fragment provided). – Clifford May 31 '15 at 08:25
@clifford, not much has been achieved. A call to strlen has been avoided. – Paul Ogilvie May 31 '15 at 14:23

Using fgets() in a proper way

1 Answers1