hi all.

i have read a text line from a file using

fgets()

into a char array

char astr[30]

then i did

strcat(astr,"secondstring")

The output of

printf (astr)

is appearing on 2 lines instead on same line.

plz advise me wat i did wrong.
thanks.

Recommended Answers

All 20 Replies

fgets places '\n' char at the end of string. Overwrite it by zero byte (end of C-string) - that's all:

if (fgets(astr,sizeof astr,f)) {
    char* p = strchr(astr,'\n');
    if (p)
      *p = '\0';
    ...
} else { /* eof or i/o error */
    ...
}

>fgets places '\n' char at the end of string.
To the OP: ArkM's code does the right thing, but the quoted statement is incomplete. fgets preserves the newline, but not if the buffer is filled. So solutions like the following distressingly common one are incorrect:

if ( fgets ( buf, sizeof buf, stdin ) != NULL ) {
  buf[strlen ( buf ) - 1] = '\0';

  /* ... */
}

ArkM's method is sufficient, but i prefer this way.

buf[strcspn(buf,'\n')] = '\0';

it's just a single line of code that will replace the newline with a NULL, if a newline is found, or will do nothing if one is not there

.

>ArkM's method is sufficient, but i prefer this way.
Keep in mind that of the potential ways to do this, strcspn is among the least efficient while strchr is among the most efficient.

oh, i wasnt aware of that.

i guess the only time i ever concern myself with reducing cycle times is in embedded applications, and there i rarely deal with strings and newlines.

but thats useful to know. thanks.

I prefer using strcspn myself, but knowing the performance characteristics is important. However, none of the methods for stripping a newline will change the fact that the whole operation is I/O bound because of fgets. You can save cycles when stripping the newline, but it isn't likely to have a noticeable effect on performance as a whole.

Just food for thought.

Narue,
Because fgets() reads a line up to the \n and loads everything up to the RETURN, isn't it simply a matter of testing the last character in the buffer with \n and if it isn't, they entered more than the requested data? Simple use of strlen() and a test?

Narue,
Because fgets() reads a line up to the \n and loads everything up to the RETURN, isn't it simply a matter of testing the last character in the buffer with \n and if it isn't, they entered more than the requested data? Simple use of strlen() and a test?

Yes, what's your point? If all you want to do is determine if a partial line was read, then looking for a newline is trivial. Ideally one would write code that can handle both cases where fgets reads a newline and where it doesn't. For example:

#include <stdio.h>

int main ( void )
{
    char buffer[5]; // Arbitrary small size
    char *s = NULL;
    size_t n = 0;

    while ( fgets ( buffer, sizeof buffer, stdin ) != NULL ) {
        size_t len = strlen ( buffer );
        char *save = realloc ( s, n + len + 1 );

        if ( save == NULL ) {
            perror ( "Insufficient memory" );
            break;
        }

        strcpy ( save + n, buffer );
        s = save;
        n += len;

        if ( buffer[len - 1] == '\n' ) {
            s[n - 1] = '\0';
            break;
        }
    }

    printf ( ">%s<\n", s != NULL ? s : "(null)" );
    free ( s );

    return 0;
}

But if you're not worrying about long lines, you still need to consider the case where a newline may not be present, which is what I was talking about with the usual naive strlen solution:

buffer[strlen ( buffer ) - 1] = '\0';

Unless this is qualified with a test to verify that a newline is present at that index, you would be removing valid data. An extra test is needed to make it work for long lines:

size_t last = strlen ( buffer ) - 1;

if ( buffer[last] == '\n' )
    buffer[last] = '\0';
commented: this post needs to be green +8

>Simple use of strlen() and a test?
strlen() has to itinerate through the string to give the length, to that
the check has to be performed to find that '\n'.

strchr() has to itinerate through the string looking for the given char.
Hard for me to see which is faster.

Yes, what's your point?

Simply to verify my thoughts on the subject.

commented: Bravo! +19

>Simply to verify my thoughts on the subject.
Okie dokie.

This thread reminded me of an old kinda-related one elseweb.

> printf (astr)
NEVER do this!
http://en.wikipedia.org/wiki/Format_string_vulnerabilities

The format string of printf/scanf must always be under your control, ideally in read-only memory.

All someone would have to do with your code is feed it some lines with % chars, and watch the fun begin!

commented: good point. +9
commented: Be warned! +16
Member Avatar for dmachop

instead of using fgets()
use fgetline()
It's much better than fgets()....................
everything2.com/title/fgetline%2528%2529
In order to get from the user type "stdin" instead of the filepointer fp.

>It's much better than fgets()....................
It's just fgets and a method for stripping the newline rolled up into a function with a largely useless return value. If you returned the length of the string, that would be so much better I could agree with your statement.

>It's much better than fgets()....................
It's just fgets and a method for stripping the newline rolled up into a function with a largely useless return value. If you returned the length of the string, that would be so much better I could agree with your statement.

May I ask why the length of the string and not the string itself or NULL if success reading was not achieved?

>May I ask why the length of the string and not the string
>itself or NULL if success reading was not achieved?
Um, what? I don't think you asked what you intended to ask.

>May I ask why the length of the string and not the string
>itself or NULL if success reading was not achieved?
Um, what? I don't think you asked what you intended to ask.

I'm sorry you didn't understand the question. Nevertheless, I asked as I intended.

You said: "If you returned the length of the string, that would be so much better I could agree with your statement."

And in other words I said if you would allow me to ask you why would you choose to return the length of the string, instead of returning a pointer to the string that has been worked on, or a NULL if not successful.

>And in other words I said if you would allow me to ask you why
>would you choose to return the length of the string, instead of
>returning a pointer to the string that has been worked on, or a NULL if not successful.
That's slightly different from how I interpreted your question, which strongly implied that I wrote the fgetline function (which I didn't).

However, to answer your question, here are some reasons for why I would choose to return the length of the string or a suitable "impossible" length:

  • fgetline, like fgets, takes a pointer to the buffer as an argument, so returning a pointer to the buffer is nothing more than a convenience. In the case of fgets, conventional usage nearly always results in the return value being used strictly for a success/failure test. Because the value could be NULL, you can't safely do something like this:
    fputs ( fgets ( buffer, sizeof buffer, stdin ), stdout );

    As such, the majority of the convenience is lost and if a better return value can be found, it's a good argument for not using a pointer to the buffer.

  • In just about every case, the length of the string is a useful entity. When you find yourself calling fgets and then shortly (or immediately) thereafter calling strlen, it makes a lot of sense for an fgets wrapper to return the result of strlen and save you the trouble.

    Returning the length of the string also doesn't preclude the current conventional usage of fgetline. The exact same success/failure test can be used:

    if ( fgetline ( buffer, sizeof buffer, in ) != -1 ) {
      /* ... */
    }

    Or if one chooses to make it a standard interface with simply a negative failure result rather than a strict -1:

    if ( fgetline ( buffer, sizeof buffer, in ) < 0 ) {
      /* ... */
    }
  • fgetline already calls strlen as a part of its internal behavior, so there's no extra overhead by changing the return type.
  • fgetline already returns an integer, so the change required to return a length is minimal:
    int fgetline ( char *str, int n, FILE *in )
    {
      int last;
    
      if ( fgets ( str, n, in ) == NULL )
        return -1;
    
      last = strlen ( str ) - 1;
    
      if ( str[last] == '\n' )
        str[last] = '\0';
    
      return last;
    }

My reasoning is that the usefulness of a string length outweighs the usefulness of a pointer that you already have.

>My reasoning is that the usefulness of a string length outweighs the usefulness of a pointer that you already have.

I appreciate your explanation of why you would endorse it. Thank you.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.