fgets() and strlen() confusion

Ok I am puzzled once again. I just don't understand how C and these functions work. I am trying to find the length of a string that is input. Two problems show up: 1) It appears fgets() is storing a newline unless I input all 8 characters and 2) strlen() is counting 1 more char than there are except when I reach the max of the string and then it counts the correct number of characters. How do you prevent fgets() from storing newline? To me this should be so trivial yet it is confusing me. Any help is appreciated. Thank you.

Code:

---

    char str[8];
    printf("Enter text: ");
    fgets(str, sizeof(str), stdin);
    printf("\nThe string %s has a length of %u.\n", str, strlen(str));

---

Output 1:
Enter text: CAFE


The string CAFE
has a length of 5.
Press any key to continue . . .

Output 2:
Enter text: 12345678


The string 1234567 has a length of 7.
Press any key to continue . . .

fgets does store a newline character in the string, as you correctly noted. strlen counts all the characters in a string, including the newline, so "CAFE\n" actually is 5 characters long.

If you want to remove the newline character, check that the last character in the string is a newline and then overwrite it with a null terminator ('\0').

Thanks got it. And just to confirm fgets() is better than scanf() right??

Yes. Much better -- because it's safer.

Quote:

---

Originally Posted by WaltP

Yes. Much better -- because it's safer.

---

scanf() can be made safe by using the conversion specifiers in your code -

Code:

---


//let's say...
char s[20];
memset(s, '\0', sizeof(s));

scanf("%10s",s); //Read max 10 digits
printf("%s",s);

---

Quote:

---

Originally Posted by Click_here

scanf() can be made safe by using the conversion specifiers in your code -

Code:

---


//let's say...
char s[20];
memset(s, '\0', sizeof(s));

scanf("%10s",s); //Read max 10 digits
printf("%s",s);

---

---

Not necessarily always safe, but certainly safer.

What about this case:

Code:

---


char *str;
size_t len;

/*Read len from the user whichever way you like */

str = malloc(len);

/* later on */

scanf("%10s",str);

---

Quote:

---

Originally Posted by claudiu

Not necessarily always safe, but certainly safer.

What about this case:

---

That is a bug that is not due to the use of scanf with the field width correctly specified, hence the use of scanf in your example is indeed safe, once that bug has been fixed. In fact, if you want to blame scanf for the bug rather than the programmer, then indeed there is nothing that can be substituted that will be safe. Absolutely nothing, ever, other than the programmer.

I don't want to blame anyone for anything. :)
I was just offering some food for thought that you need to be aware of what you are doing when using scanf. Of course there is a bug in there, that was the whole point, to show that you can trip yourself with scanf even if you specify the size of the string you are reading in. The question originally was related to the ancient fgets() vs scanf debate and the fact that fgets is so pedantic and requires a size parameter makes it safer, because it makes you, the programmer, immediately aware of potential overflow problems.

Quote:

---

Originally Posted by claudiu

I was just offering some food for thought that you need to be aware of what you are doing when using scanf.

---

Certainly, but you need to be aware of what you are doing when you are programming. The use of fgets is no real improvement if you are careless, e.g.,

Code:

---

char *str;
size_t len;

/*Read len from the user whichever way you like */

str = malloc(len);

/* later on */

fgets(str, len, stdin);

---

The above code is vulnerable to buffer overflow because len was doubled between the calls to malloc and fgets.

Likewise, we can "fix" the "problem" with scanf in your (counter-)example by making it such that the field width is not hard coded into the format string, but then the resulting code will be equally vulnerable to buffer overflow as in my scenario.

I think that saying along the lines of the programmer might introduce a bug with the scanf statement, so it should be avoided is not right.

I can introduce the same bug using a "for" or a "while" statement, but you wouldn't even think of saying, "avoid for and while loops".

You can use scanf() super safely but it can be a pain in the ass.

Code:

---

#include <stdio.h>
#include <stdlib.h>
int main()
{
    char *data = NULL;
    char format[16];
    int length = 0;
    int convert = 0;
    puts("Please enter a string length.");
    convert = scanf("%i", &length);
    if (convert == 1 && length > 1)
    {
        data = malloc(1 + length);
        sprintf(format, "%%%ds", length);
    }
    else
    {
        fputs("Bad string length entered.\n",stderr);
        return 1;
    }

    printf("OK, enter your string (length = %d).\n", length);
    convert = scanf(format, data);
    if (convert == 1)
    {
        printf("Good job.\n\"%s\"\n", data);
    }
    else
    {
        fputs("There was a problem.\n",stderr);
    }
    free(data);
    data = NULL;

    return 0;
}

/*
Please enter a string length.
20
OK, enter your string (length = 20).
Low-calorie.beer
Good job.
"Low-calorie.beer"
*/

---

Basically you need to do that. I guess it could look worse, and I didn't check malloc(). Oh well. There's annoying things about scanf that won't be fixed, like not reading strings longer than one word.

[edit] It's not that scanf is a bad function that works poorly, but user input is frequently not formatted at all. Use scanf to read formatted files or something. It can be a lot of work to shoehorn this one way you know to get input into the program securely.[/edit]

Quote:

---

Originally Posted by whiteflags

Basically you need to do that. I guess it could look worse, and I didn't check malloc(). Oh well. There's annoying things about scanf that won't be fixed, like not reading strings longer than one word.

---

Yes, it is annoying to create a format string for scanf, but scanf does have a way to read arbitrary strings, including whitespace, using the '[' format specifier. A common format is "%[\n]" (or something like "%20[\n]" with a maximum length).

I do prefer using fgets, though, since users in general expect the program to read whole lines at a time. sscanf can then be used on each input line string if needed.