11
\$\begingroup\$

After a successful fgets(buffer, ...), it is often desirable to trim a potential End-of-Line '\n'.

Of the following 2 methods, are there any shortcomings?

#include <stdio.h> char buffer[100]; // Method 1 while (fgets(buffer, sizeof buffer, stdin) != NULL) { size_t len = strlen(buffer); if (len > 0 && buffer[len - 1] == '\n') buffer[--len] = '\0'; // use buffer printf("\"%s\"\n", buffer); } // Method 2 while (fgets(buffer, sizeof buffer, stdin) != NULL) { strtok(buffer, "\n"); // use buffer printf("\"%s\"\n", buffer); }

Notes:

  1. When reading text files from alternate file systems, strtok(buffer, "\r\n") looks useful.
  2. strtok() may incur an issue with another strtok() sequence.
\$\endgroup\$
4
  • \$\begingroup\$ Could you please make this into a short runnable program with examples of how you have tested it and expect it to run? And could you expand what you mean by "incur an issue with another strtok() sequence"? \$\endgroup\$ Commented Oct 22, 2014 at 22:23
  • \$\begingroup\$ @syb0rg strtok() is not re-entrant. There is a strtok_r() variant that is re-entrant. \$\endgroup\$ Commented Oct 22, 2014 at 22:40
  • \$\begingroup\$ @200_success I know, I thought that is what he meant but just wanted to confirm. Note that strtok_r is provided by the POSIX standard, and that strtok_s is the C11 standard version. \$\endgroup\$ Commented Oct 22, 2014 at 22:44
  • \$\begingroup\$ @syb0rg 1) In re: "incur an issue with another strtok() sequence": If code was in the middle of calling a series of strtok() to perform some other tokenizing, then this use of strtok() would fouls things up. That does not seem likely, but is a down side. 2) Thanks 1K 3) Did not know about strtok_r() 4) strtok_s() in in the normative "Bounds-checking interfaces" section of C11. \$\endgroup\$ Commented Oct 23, 2014 at 2:56

4 Answers 4

Reset to default
8
\$\begingroup\$

For what it is worth, using strlen is much more efficient than using strtok. I did some tests (omitting the file access) using strlen, strchr, strtok, strstr, strpbrk and strcspn:

It surprised me that to execute this neat looking thing:

s = strchr(s, '\n'); if (s) { *s = '\0'; }

takes nearly 50% longer than this ugly looking sucker:

size_t len = strlen(s); if (len && (s[len-1] == '\n')) { s[len-1] = '\0'; }

and the beautiful

strtok(s, "\n\r");

and

strsep(&s, "\n\r");

are 10 to 20 times slower. The latter two do of course look for \r as well as \n. But even adding \r, the strlen approach is still quicker than the strchr and still quicker then strtok and strsep by 10 to 20 times:

size_t len = strlen(s); if (len && ((s[len-1] == '\n') || (s[len-1] == '\r'))) { s[len-1] = '\0'; }

Note that I wasn't exhaustive with string variations. In fact I didn't even break a sweat. And of course this is all quite irrelevant as the stdio call will dominate in your target code :-)

Note also from the comments below that if the first character of the search string (s in the examples above) is one of the characters in the pattern string, strtok does nothing (thanks to @chux).

\$\endgroup\$
3
  • \$\begingroup\$ Recently found out strtok(s, "\n\r"); does not always trim the '\n' from s. When s == "\n". The '\n' remains. \$\endgroup\$ Commented Jan 1, 2015 at 9:05
  • 1
    \$\begingroup\$ @chux I tried it on a Mac and you are right - which system did you use? I'm shocked - it is rather like finding out that an old friend has a dark secret. And it is not specific to \n - if any char in the search pattern appears as the first char in s, strtok does nothing. My man page doesn't describe this behaviour. Another reason not to use strtok - strsep works as expected. \$\endgroup\$ Commented Jan 1, 2015 at 14:17
  • \$\begingroup\$ Might want to add this about strtok() as relates to "are there any shortcomings?" Looked into char *ptr = buffer; strsep(ptr, "\n") and did not find any functional shortcomings, other than it is non-standard. \$\endgroup\$ Commented Jan 1, 2015 at 16:37
4
\$\begingroup\$

Why not use the scanf() functionality do the work for you.

char buffer[100]; int len; if (fscanf(stdin, "%99[^\n]%n", buffer, &len) != 1) { // No data (or just a '\n') len = 0; } fscanf(stdin, "%*[^\n]"); /* If the line is longer than 99 this throws away * The rest of the line upto the new line character */ buffer[len] = '\0'; fscanf(stdin, "%*c"); /* read and throw away the newline */
\$\endgroup\$
3
  • \$\begingroup\$ Interesting idea this different approach. Forming the needed width of "99" is not much an issue when the buffer size is well know (as in my example), but forming the width when the size is not a literal constant need more code. Doable, but cumbersome. \$\endgroup\$ Commented Oct 23, 2014 at 3:04
  • \$\begingroup\$ @chux: Not really that cumbersome if you wrap it in a function. \$\endgroup\$ Commented Oct 23, 2014 at 4:35
  • \$\begingroup\$ Could use int len = 0; fscanf(stdin, "%99[^\n]%n", buffer, &len); and skip the if() test for same functionality. \$\endgroup\$ Commented Jul 12, 2017 at 13:14
4
\$\begingroup\$

For completeness, I tried a few alternative ways of doing the file access. I tried your fgets and the suggested alternatives from other answers.

On my system, using fgets, which you used, is about the same speed as using getline. Using fscanf is about 3x slower than these solutions. The fastest method I tested was to use fgetln (available on a Mac from the BSD C library), which was 10-15% faster than fgets/getline. 

char *line = NULL; size_t len = 0; while ((line = fgetln(f, &len)) != NULL) { if (line[len - 1] == '\n') { line[len - 1] = '\0'; } //printf("\"%s\"\n", line); } if (ferror(f)) { perror("fgetln"); }

Another alternative is to use mmap to map the file (only real files though, not pipes, etc) into memory and use string handling directly (strchr etc) to look for line endings. This might be the fastest of all but I didn't test it here.

Generally it is best not to get too hung up about speed unless you know that a piece of code is critical (and normally one doesn't know this). So think more about the advantages and disadvantages of the various solutions:

  • fgets is quick but uses a fixed buffer length and you have to handle overflow as a special case.

  • getline and fgetln are quick, elegant and capture complete lines. These are attractive if we do not need to worry about attacks by huge strings. fgetln is not in any standard so is rather less portable than getline (which seems to be POSIX). I tried fgetln with a file containing one line > INT_MAX chars long (INT_MAX is the limit that fgetln handles) and although it did slow down the system considerably, it failed cleanly with a memory allocation failure:

    fgetln: Cannot allocate memory

    getline has a limit of SSIZE_MAX which is much bigger.

  • fscanf - I can't think of a reason to use this for the simple example you pose, but it has its uses if you want to control the input format.

  • mmap is (probably) quick and elegant but only handles real files, which restricts the application a bit (eg you can't just pipe input into the application if it wants to map a file).

\$\endgroup\$
1
  • \$\begingroup\$ Oops, you are right. My man page says it is in the Standard C Library (libc, -lc), but I guess that does not mean it is in the C standard as the history section says only "The fgetln() function first appeared in 4.4BSD.". I'll edit the answer... \$\endgroup\$ Commented Oct 24, 2014 at 19:13
3
\$\begingroup\$

It seems silly to have to call strlen() or strtok() to find the end-of-line delimiter, when surely the system could have already known how many bytes were successfully read into buffer.

Also consider how you want to handle the case where the input line is too long for the buffer.

To take care of both issues, I suggest using getline() instead:

char *line = NULL; size_t buflen = 0; ssize_t len; while ((len = getline(&line, &buflen, stdin)) > 0) { if (line[len - 1] == '\n') { line[--len] = '\0'; } // use buffer printf("\"%s\"\n", line); }
\$\endgroup\$
3
  • 1
    \$\begingroup\$ I look forward to using getline() more with its allocated buffer once it becomes part of the C standard. Of course one could roll their own getline() in the mean time. A niche area concern that I have with getline() is the inability to limit the line length. A limit is useful to prevent hostile input from creating some huge input aimed to overwhelm code resources. \$\endgroup\$ Commented Oct 23, 2014 at 3:15
  • \$\begingroup\$ How would you want the program to behave in the face of unreasonably long input lines? Your two original methods avoid exhaustion, but they don't feel quite correct either. \$\endgroup\$ Commented Oct 23, 2014 at 3:16
  • \$\begingroup\$ Various approaches: 1) consume input up to a point and toss the rest, then report the error. (prevents OOM) 2) consume and process input up to a limit, recognize it as an attack and report the error (prevent OOM and spinning the CPU) \$\endgroup\$ Commented Oct 23, 2014 at 3:24

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.