Thread: strcpy get errors

Originally Posted by Barney McGrew

Do you see any potential errors here?

Code:

char c;
if (ungetc('\n', stdin) != EOF)
        gets(&c);

As a matter of fact, yes. If there has been a preceding call of ungetc(), and that character has not been read, the next call of ungetc() leaves non-binary streams in an undefined state.

In any event, can you name a circumstance in which that particular construct would achieve anything useful .... in comparison, say, with "c = '\n';"

Originally Posted by Barney McGrew

I don't know about this. If you look at the outline of major changes in n1570.pdf, you'll see this:

n1570 is a draft. It is not the standard. As a matter of policy, standards committees do not remove features from standards that were in previous versions of a standard and have been in common use, without a process of deprecation (deprecate in one or more version of the standard, before removing it).

If there has been a preceding call of ungetc(), and that character has not been read, the next call of ungetc() leaves non-binary streams in an undefined state.

All I'm aware of is:
• Only one call to ungetc is guaranteed to succeed.
• If ungetc fails (perhaps due to multiple calls to ungetc), then it returns EOF.
• Calls to ftell or fgetpos may fail for text streams if there are any pushed back characters in the stream.

I think you're referring to the third point, but that's only a problem with *other* calls to ungetc, since gets consumes the pushed back character immediately.

In any event, can you name a circumstance in which that particular construct would achieve anything useful .... in comparison, say, with "c = '\n';"

I suppose it can be used to consume a sequence of characters that have been pushed back up to a new line. Either way, my contention is that it's possible to use it safely, not that it can be used safely in a useful way.

n1570 is a draft. It is not the standard.

I don't have a copy of the standard, unfortunately. Do you believe the standard differs from the draft on this?

edit: They list many things as deprecated in the draft, but they specifically say that gets is removed.

Originally Posted by Barney McGrew

I suppose it can be used to consume a sequence of characters that have been pushed back up to a new line.

Which is unsafe, as gets() would write more than one character to your one character buffer.

Originally Posted by nonpuz

Yeah, it was replaced by a "safer" alternative gets_s which takes a size parameter.

As a Microsoft, vendor specific, extension. Not by the standard.

gets() is not being replaced since a workable alternative - fgets() - already exists.

Originally Posted by grumpy

As a Microsoft, vendor specific, extension. Not by the standard.

gets() is not being replaced since a workable alternative - fgets() - already exists.

I read about gets_s in the C11 draft - But as you said earlier, it's a draft. And point 6 for the gets_s function in that draft recommends the use of fgets instead of get_s anyway.

Originally Posted by Barney McGrew

gets is only as unsafe as strcpy is, in that it will mess up if it's used incorrectly.

No. strcpy can be used safely since the source string can be guaranteed to be of a length such that it can be copied to the destination without buffer overflow. gets cannot be made safe since the source string's length cannot be guaranteed, unless the user is aware and promises to cooperate (or you use your silly ungetc idea, which basically means that the whole gets call was pointless to begin with). Of course, this does not mean that strcpy is not a potential source of buffer overflows since it is easy to use incorrectly, but to say that it is "only as unsafe as strcpy is" is mistaken.

EDIT:

Originally Posted by Click_here

Strncpy is your best bet with a definition for the maximum string length.

A catch with strncpy is that you must remember to null terminate, or write/use a wrapper that does so.

Just following on from laserlight -

"strcpy" does not have to have external input as it's arguments, "gets" does.

Originally Posted by nonpuz

I was referring to the C11 draft, _NOT_ the microsoft garbage.

But that is exactly the point. The draft includes a number of things that various contributors advocated for the standard, including Microsoft lobbying for those "safe" functions. Other contributors lobbied for addition or removal of other features. Quite a few things were reflected in the draft, but did not make it into the final version.

Yes maybe in future I will use fgets or whatever, but gets(string) is pretty easy to remember and simple to use.

I would say in reality a bug from too much input is going to be pretty easy to spot.

I suppose you could have a getsn where you provided a max length ie getsn(string, max_length).

What it could also do is malloc the memory for the string itself and return a pointer to the string.
Then to avoid memory leakage free the previous string ( or over write it) on the next gets() call, if any.
And deallocate it on program termination (which is done anyway I believe).

That is how I would have done it if I was writing the C standard, but then they never asked me
so they only have themselves to blame

They also never asked me to write the the editor for posting post which is why the formatting is all screwed up, but that's another story!!


[eta the formatting problem only seems to show up in the preview box, why they used a different sized box is anyone guess, probably too busy butting their fgets(buff, 100 stdin)'s in!!

Originally Posted by esbo

I would say in reality a bug from too much input is going to be pretty easy to spot.

That's why Matticus and Click_here spotted it when they replied to you in posts #36 and #37, respectively

(EDIT: jokes aside, your assertion does not hold up to the fact that mistakes that allow for buffer overflow are actually more common than we would like. Such mistakes may be easy to spot on hindsight, but not so when you're coding or doing a review with other things to look out for.)

Originally Posted by esbo

I suppose you could have a getsn where you provided a max length ie getsn(string, max_length).

What it could also do is malloc the memory for the string itself and return a pointer to the string.
Then to avoid memory leakage free the previous string ( or over write it) on the next gets() call, if any.

It is not standard C, but the POSIX standard getline function does something like what you have in mind, except that it is a single function rather than two.

Originally Posted by esbo

And deallocate it on program termination (which is done anyway I believe).

It might be, but it is good practice to explicitly call free for what you malloc (directly or indirectly, e.g., with getline)

Originally Posted by grumpy

Which is unsafe, as gets() would write more than one character to your one character buffer.

Obviously you'd increase the size of the buffer as it's needed. With the example I presented, only one character is necessary in order to store the '\0' character. If you know that there are exactly three characters to be read from the stream before the new line character, you'd allocate an array of four chars and pass its pointer to gets.

You can never, ever allocate enough memory to make gets() a safe call

The example I provided uses gets. Is it unsafe?

or you use your silly ungetc idea, which basically means that the whole gets call was pointless to begin with

In which case it's used correctly, providing you agree that a correct program works in a predictable way and an incorrect one invokes undefined behaviour.

I'm not really arguing in favour of using gets since, as I think most of you would agree, it's commonly used to write buggy programs. I just think it's silly to say that gets, as a whole, is unsafe when it's more accurate to say that it's commonly used in an unsafe way.

Also, on a slightly unrelated topic, using the "s" format specifier with scanf without a maximum field length has the same issue that gets has, but strangely, no one has seemed to comment on that in this thread when its use has been suggested on a few occasions.

edit: Some have suggested using strncpy instead of strcpy, which usually isn't a good idea. strncpy's designed specifically for copying data into a fixed 'field' for files and networks, that's why it doesn't terminate with '\0' if the size of the source string is equal to the size parameter, and why it pads the remainder of the storage with '\0's when it's less. The only real alternative you have, in standard C, is snprintf (or just strcpy after verifying the size of the source string with strlen), but some implementations provide strlcpy, strdup, and asprintf.

Originally Posted by Barney McGrew

Obviously you'd increase the size of the buffer as it's needed. With the example I presented, only one character is necessary in order to store the '\0' character. If you know that there are exactly three characters to be read from the stream before the new line character, you'd allocate an array of four chars and pass its pointer to gets.

How would you know that "there are exactly three characters to be read from the stream before the new line character", without making the use of gets redundant?

Originally Posted by Barney McGrew

The example I provided uses gets. Is it unsafe?

No, but it is pointless. You are trying to use a contrived scenario to debunk a statement that is true except for this silly scenario. In such a case, you might win the argument on a technicality, but the win would be pointless in reality.

Originally Posted by Barney McGrew

In which case it's used correctly, providing you agree that a correct program works in a predictable way and an incorrect one invokes undefined behaviour.

If the only correct use of gets is to do that, then that means that gets should not be used since simple assignment is exceedingly better. I can only describe that "correct use" as brain damaged.

Originally Posted by Barney McGrew

Also, on a slightly unrelated topic, using the "s" format specifier with scanf without a maximum field length has the same issue that gets has, but strangely, no one has seemed to comment on that in this thread when its use has been suggested on a few occasions.

Not strange at all: people miss such mistakes. I have corrected people on this in the past, and I also have failed to correct people when I did not notice it. Either way, you can specify the format specifier correctly.