strcpy_s vs strcpy
in [C++]
|
Prev: composition and bad_alloc
Next: ANN: Library dependency graphs tool status update and questions
From: Martin Bonner on 2 Jul 2008 02:58 On Jul 2, 12:33 pm, Trups <Samant.Tru...(a)gmail.com> wrote: > Hi, > > I have changed my strcpy to strcpy_s for 2005 project. Notes: 1. strcpy_s is a vendor unique extension. It is not known to the C++ standard 2. std::string just "Does The Right Thing". I would strongly suggest that if you are going to rewrite your code, you do so to use std::string. Having said all that: > It's fairly > big project and was using strycpy lot of places. > The program started corrupting the stack and in turn crashing the > application. We have realized that it is due to strcpy_s. We have > changes that to strpcy and then it was fine. > There are some places the destlength was more then whatever size of > deststr. I know it is a mistake but the copy string had character to > copy. So I was thinking it shouldn't crash the project. Isn't that > true? No. strcpy_s deliberately writes to every character of the buffer that you claim to have supplied it. > > Example: > deststr[128]; > copystr[] = "Test String"; > destlength = 256; > > strcpy_s(deststr, destlength, copystr); "If you lie to the run-time, it will get its revenge." The rationale for writing to every character of the buffer is as follows: Consider when copystr is not a fixed array, but instead some user supplied data. Your testing will use "reasonable" values of the data, and won't have any problems. The bad guys will supply "unreasonable" values, and overwrite the stack giving them control of your code ... except that strcpy_s overwrites /all/ of the buffer, and so you find the problem in testing. > even strcpy_s(deststr, strlen(copystr), copystr); was crashing (I need > to look more for this) That surprises me. Are you sure that copystr was not actually too long to fit in the buffer? > But is there any known problem with strcpy_s? Not really - just in your usage. I reiterate - fix your code to use std::string. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Greg Herlihy on 2 Jul 2008 17:33 On Jul 2, 4:33 am, Trups <Samant.Tru...(a)gmail.com> wrote: > I have changed my strcpy to strcpy_s for 2005 project. It's fairly > big project and was using strycpy lot of places. It might helpful to provide a little background information here - since many C++ programmers are probably not familiar with "strcpy_s()", and the other routines described in the ISO C Committee's Technical Report 2. The TR2 document (a draft of which can be found on the ISO C Committee's web site): http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1225.pdf describes a "bounds-checking" interface to the Standard C Library. > The program started corrupting the stack and in turn crashing the > application. We have realized that it is due to strcpy_s. We have > changes that to strpcy and then it was fine. The primary benefit of replacing strcpy() with strcpy_s() calls in a program, is forcing the programmer to document the size of each destination buffer that is used in a copy string operation. So the sensible fix would be to specify the size of the destination buffer accurately in each call to strcpy_s - rather than to specify no size at all. Reverting the calls to strcopy_s() back to strcpy() does nothing to mitigate the danger of buffer overruns, instead, the only effect is to sweep the entire issue under the rug. > There are some places the destlength was more then whatever size of > deststr. I know it is a mistake but the copy string had character to > copy. So I was thinking it shouldn't crash the project. Isn't that > true? No. Calling strcpy_s() is likely to fill the entire destination buffer with bytes - regardless of the length of the string being copied. On modern processors, copying the bytes into the destination buffer and checking the string for a nul character - can be completed much more quickly as two, independent operations than they can be when combined as a single operation. As the footnote in the ISO document explains: "This allows an implementation to copy characters from s2 to s1 while simultaneously checking if any of those characters are null. Such an approach might write a character to every element of s1 before discovering that the first element should be set to the null character." > Example: > deststr[128]; > copystr[] = "Test String"; > destlength = 256; > > strcpy_s(deststr, destlength, copystr); The program has to ensure that size of destsr is correctly specified in every call to strcpy_s(). Therefore when copying a string to a fixed-sized char array, sizeof() should always be used to specify the size of the destination buffer: strcpy_s( deststr, sizeof(destr), copystr); > even strcpy_s(deststr, strlen(copystr), copystr); was crashing (I need > to look more for this) The length of the "copystr" string clearly has no relation to the size of the deststr buffer. So, regardless whether the above strcpy_s() call crashes or not - the call is simply not correct. Indeed, passing strlen(copystr) as the size of the destr buffer defeats the entire purpose of using a bounds-checking interfaces in the first place. Bounds cannot be checked unless they are accurately specified. > But is there any known problem with strcpy_s? Not when used correctly. Greg -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Mathias Gaunard on 2 Jul 2008 18:28 On 2 juil, 13:33, Trups <Samant.Tru...(a)gmail.com> wrote: > Hi, > > I have changed my strcpy to strcpy_s for 2005 project. It's fairly > big project and was using strycpy lot of places. Then it's probably a C project, not a C++ one. > The program started corrupting the stack and in turn crashing the > application. We have realized that it is due to strcpy_s. We have > changes that to strpcy and then it was fine. > There are some places the destlength was more then whatever size of > deststr. I know it is a mistake but the copy string had character to > copy. So I was thinking it shouldn't crash the project. Isn't that > true? > > Example: > [char ]deststr[128]; > [char ]copystr[] = "Test String"; > [int ]destlength = 256; > > strcpy_s(deststr, destlength, copystr); [] added by me. You're not supposed to pass a size bigger than your buffer to strcpy_s. Still, it should work perfectly in that case. > even strcpy_s(deststr, strlen(copystr), copystr); was crashing (I need > to look more for this) 11 bytes is obviously not enough to write 12 bytes. Still, that shouldn't crash. You may corrupt the stack when reading till null though. What you want is strcpy_s(deststr, sizeof deststr, copystr); > But is there any known problem with strcpy_s? Apart from the part it's perfectly useless Microsoft-specific monstruosity and duplicates the standard strncpy, I do not think there is any. You obviously didn't even provide code that compiles and that exhibits your problem. You should demonstrate your problems with a testcase. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Francis Glassborow on 2 Jul 2008 18:28 Thomas Richter wrote: > Trups schrieb: > >> I have changed my strcpy to strcpy_s for 2005 project. It's fairly >> big project and was using strycpy lot of places. >> The program started corrupting the stack and in turn crashing the >> application. We have realized that it is due to strcpy_s. We have >> changes that to strpcy and then it was fine. > > First, strcpy_s is a microsoftism. Don't use it for portable programs. Not entirely true. MS originated it but it is part of one of the recent C TRs. -- Note that robinton.demon.co.uk addresses are no longer valid. [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Martin Bonner on 2 Jul 2008 23:58
On Jul 3, 10:28 am, Mathias Gaunard <loufo...(a)gmail.com> wrote: > On 2 juil, 13:33, Trups <Samant.Tru...(a)gmail.com> wrote: > > But is there any known problem with strcpy_s? > > Apart from the part it's perfectly useless Microsoft-specific > monstruosity and duplicates the standard strncpy, I do not think there > is any. Except that strncpy does not reliably nul-terminate the buffer, and strncpy is defined to zero pad buffer (which can be a performance issue for small strings copied to large buffers). strcpy_s /does/ reliably nul-terminate, and is not required to write to the whole of the buffer (although it may do for reliability reasons). -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |