strcat_s problems

This is a discussion on strcat_s problems within the C++ Programming forums, part of the General Programming Boards category; I'm writing a function that combines first, middle, and last names into another array. Using strcat and strcpy in the ...

  1. #1
    Registered User
    Join Date
    May 2009
    Posts
    242

    strcat_s problems

    I'm writing a function that combines first, middle, and last names into another array. Using strcat and strcpy in the following code, everything works fine (they obviously don't take the integer in the middle). Also, the definitions in the main program are:

    char First[15], Middle[15], Last[15];

    Here's the function that's giving me problems:

    Code:
    char * combineNames(const char *First, const char *Middle, const char *Last)
    {
    	char *FullName = new char[46];
    
    	strcpy_s(FullName, 15*4, Last);
    	strcat_s(FullName, 15*4, ", ");
    	strcat_s(FullName, 15*4, First);
    	strcat_s(FullName, 15*4, " ");
    	strcat_s(FullName, 15*4, Middle);
    
    	return FullName;
    }
    I'm really swimming on the integer to put in there. I tried smaller ones and got error messages (or crashes), and these were just my last experimental values before turning to you guys. The message I get when I run the program goes something like: "application wrote to memory after end of heap buffer" and it gives a buffer location, too.

    I also think the number in the middle is supposed to be a number of bytes rather than number of places in the array.

    As I say, using strcpy and strcat with no integer, the whole thing works fine, but it looks like those are deprecated according to warnings in MS suggesting using strcpy_s instead.

  2. #2
    Registered User
    Join Date
    May 2009
    Posts
    242
    just for clarity, this is the version that works (but gives warning messages when compiling):

    Code:
    char * combineNames(const char *First, const char *Middle, const char *Last)
    {
    	char *FullName = new char[46];
    
    	strcpy(FullName, Last);
    	strcat(FullName, ", ");
    	strcat(FullName, First);
    	strcat(FullName, " ");
    	strcat(FullName, Middle);
    /*	
    	strcpy_s(FullName, 15*4, Last);
    	strcat_s(FullName, 15*4, ", ");
    	strcat_s(FullName, 15*4, First);
    	strcat_s(FullName, 15*4, " ");
    	strcat_s(FullName, 15*4, Middle);
    */
    	return FullName;
    }

  3. #3
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,594
    Quote Originally Posted by Aisthesis
    As I say, using strcpy and strcat with no integer, the whole thing works fine, but it looks like those are deprecated according to warnings in MS suggesting using strcpy_s instead.
    Microsoft is deprecated. By that I mean that I do not recommend heeding Microsoft's statement that those functions are deprecated because they are not. Perhaps you can infer that Microsoft is not actually deprecated, but rather merely recommend their versions of these functions over the standard versions.

    To put a spanner in the works, I suggest not using null terminated C style string variables. Use std::string instead, e.g.,
    Code:
    std::string combineNames(const std::string& First, const std::string& Middle, const std::string& Last)
    {
        return Last + ", " + First + " " + Middle;
    }
    or if you want to avoid excessive concatenation:
    Code:
    std::string combineNames(const std::string& First, const std::string& Middle, const std::string& Last)
    {
        std::stringstream ss;
        ss << Last << ", " << First << " " << Middle;
        return ss.str();
    }
    or if you can use Boost:
    Code:
    std::string combineNames(const std::string& First, const std::string& Middle, const std::string& Last)
    {
        return str(boost::format("%3%, %1% %2%") % First % Middle % Last);
    }
    Last edited by laserlight; 05-31-2009 at 12:13 PM.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  4. #4
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,538
    Quote Originally Posted by laserlight View Post
    Microsoft is deprecated. By that I mean that I do not recommend heeding Microsoft's statement that those functions are deprecated because they are not. Perhaps you can infer that Microsoft is not actually deprecated, but rather merely recommend their versions of these functions over the standard versions.
    I would recommend strcpy_s and strcat_s over the normal ones because portability can be achieved with a simple macro.
    As for why it doesn't work (aside from the real solution you really should be using that laserlight just mentioned), it's because you are writing beyond the buffer.
    The first doesn't work because 1) your buffer sizes are WRONG (you allocate 46 chars, yet say you have 60) and 2) because you ARE writing more data than available and the functions are trying to tell you that.
    The second doesn't work because of reason 2 above. It doesn't give you any helpful assert, but instead tells you that you clobbered (or overwrote) some memory (you shouldn't have).
    All in all, undefined behavior and often a crasher and a security risk.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  5. #5
    The larch
    Join Date
    May 2006
    Posts
    3,573
    If you don't want to use std::string, you can determine how much space is needed.

    Code:
    unsigned TotalLength = strlen(First) + strlen(Middle) + strlen(Last)
        + 3 //", " + " "
        + 1; //terminator
    char* FullName = new char[TotalLength];
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  6. #6
    Registered User
    Join Date
    May 2009
    Posts
    242
    Well, the assignment calls for using character arrays and not strings for First, Middle, Last and FullName.

    As for the buffer overruns, I got an error also when I used

    strcpy_s(FullName, 14*4, Last);
    strcat_s(FullName, 2*4, ", ");
    strcat_s(FullName, 14*4, First);
    strcat_s(FullName, 1*4, " ");
    strcat_s(FullName, 14*4, Middle);

    I'm not certain whether it was the same message, but I do remember that that didn't work--similarly when I tried not counting the null terminator and added 1 to each, similarly when I didn't multiply by 4, which I started doing because I concluded that these were supposed to be bytes rather than places in the array--the reason I came up with 15 for all was because the small values that seemed like possible right ones didn't work, so I decided to just give everything plenty of headroom, and that didn't work either. But the real point is that I'm just blindly groping here for numbers that work without understanding the principle behind it.

    So, if we restrict the problem to what numbers are required to make this work GIVEN the definitions of the arrays, what do we need to put in the middle field on each of these? And what's the general rule for figuring the number we need there?

    As to the deprecated issue, I was surprized, too, as the book I'm working from was published this year, so it should be up-to-date on what's deprecated and what isn't. Book likes using strncat, strncpy in most cases, but I figured strcpy/strcat are fine here because the array definitions guarantee that bounds can't be overstepped.

    Anyhow, I'm taking your comments to mean "don't worry about strcat/strcpy," which is fine, but I'd still like to understand the functions MS is recommending.

  7. #7
    Registered User
    Join Date
    May 2009
    Posts
    242
    As to the length of the arrays, they are determined exactly by anon's equation. Note that with First[15] maximum strlen is going to be 14 (same for the others). So, we have 14 + 14 + 14 + 3 + 1 = 46. That's why I picked 46 for the length of FullName.

    But that's not what's giving me problems. It's this mysterious integer that MS's function want and for which I don't understand how one is supposed to pick an appropriate value.

  8. #8
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,485
    > It's this mysterious integer that MS's function want
    So did you ever bother to read the manual?
    strcat_s, wcscat_s, _mbscat_s
    Code:
    errno_t strcat_s(
       char *strDestination,
       size_t numberOfElements,
       const char *strSource 
    );
    Parameters

    strDestination

    Null-terminated destination string buffer.

    numberOfElements

    Size of the destination string buffer.

    strSource

    Null-terminated source string buffer.

    Return Value

    Zero if successful; an error code on failure.
    Every call should be 46 (or whatever number you allocated)
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  9. #9
    Registered User
    Join Date
    Dec 2006
    Location
    Canada
    Posts
    3,170
    Without the additional, non-standard "size" parametre, it's not any more secure than the standard version. Might as well just use the standard version.

    I would recommend strcpy_s and strcat_s over the normal ones because portability can be achieved with a simple macro.
    If you can make it standard compliant by just using a macro, the way you are using it is not any more secure than the standard version. Might as well just use the standard version in the first place.

    The way these _s (non-standard) functions work is that, they take an additional "buffer size" parametre, so they can make sure you won't overstep it. Calling them without the buffer size paramtre (is that even allowed?) is rather pointless.

    [edit]
    Of course, the same can be achieved using standard, portable functions by checking the length of the input before passing it to strcat.
    [/edit]
    Last edited by cyberfish; 06-01-2009 at 02:56 AM.

  10. #10
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,594
    Quote Originally Posted by cyberfish
    If you can make it standard compliant by just using a macro, the way you are using it is not any more secure than the standard version.
    I believe Elysia's idea is to conditionally define macros that forward the extended versions to the standard versions if the extended versions are not provided.

    Quote Originally Posted by cyberfish
    The way these _s (non-standard) functions work is that, they take an additional "buffer size" parametre, so they can make sure you won't overstep it.
    Not quite: they make sure that if there is out of bounds access, it will be detected.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  11. #11
    Registered User
    Join Date
    Dec 2006
    Location
    Canada
    Posts
    3,170
    I believe Elysia's idea is to conditionally define macros that forward the extended versions to the standard versions if the extended versions are not provided.
    But that means, when compiled by a compiler without those extensions (for example, every compiler other than MSVC), the code won't be secure, assuming the programmer relies on the bound-checking extension.

    And then there are functions that have different signatures than the standard equivalents. Like fopen_s

    fopen_s, _wfopen_s

    As far as I can see, it makes the return value of the regular fopen a parametre (pointer to pointer), which allows it to return a more detailed error (rather than just a null pointer for all errors).

    Whether that really helps I can't tell, certainly not in terms of "security", though, but at most in convenience. Microsoft is just specializing C.

    Not so easy to fix with macros.

    And then there is scanf_s, which takes a buffer size for all "%s" parametres. Not so easy to fix with a macro either, and there is a perfectly standard and portable way to do it (just include the field width with the placeholders - "%10s"). But no, they deprecated scanf and made a new, non-compatible one.

    And then there's gets(). I agree it should be deprecated, but not for gets_s. Why not just deprecate it in favour of fgets?

    While some _s functions are probably better (in some way), than the standard functions, it gives me the impression that they are trying very hard to find any slightest reason to deprecate standard functions and replace them with their _s functions.

    There's even a rand_s(). Same deal as fopen_s. Take in the return address as a pointer, just so it can return an error value. Exactly how many different ways can rand() fail? (I don't think they deprecated rand(), though, so that's good)

  12. #12
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,594
    Quote Originally Posted by cyberfish
    But that means, when compiled by a compiler without those extensions (for example, every compiler other than MSVC), the code won't be secure, assuming the programmer relies on the bound-checking extension.
    When it comes down to it, just using those extended versions does not make the code secure anyway. It just helps to detect lapses. Relying on the bounds checking would be like relying on std::out_of_range to be thrown when using the at() member function of std::vector, as opposed to making sure that the accesses are within range and just using the bounds checking provided to be on the safe side.

    Quote Originally Posted by cyberfish
    While some _s functions are probably better (in some way), than the standard functions, it gives me the impression that they are trying very hard to find any slightest reason to deprecate standard functions and replace them with their _s functions.
    They are not deprecating the standard library functions, and in fact they have no power to do so. I repeat, when they say "deprecate", they mean "we do not recommend". To use their own language: their use of "deprecated" is deprecated.
    Last edited by laserlight; 06-01-2009 at 03:47 AM.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  13. #13
    Registered User
    Join Date
    Dec 2006
    Location
    Canada
    Posts
    3,170
    They are not deprecating the standard library functions, and in fact they have no power to do so. I repeat, when they say "deprecate", they mean "we do not recommend". To use their own language: their use of "deprecated" is deprecated.
    That's good news!

    "Deprecated" is a strong word IMHO. In GCC, a deprecated feature is one that may be removed in a later release. I certainly hope they are not going to remove std lib functions.

    Still, they are "not recommending" many standard functions, and replacing them with their own, when there are standard ways to solve the "problems" they find with standard functions.

    For a C newbie, this can cause pretty bad vendor lock-in. Most of them will think if Microsoft is deprecating a function, it's for good reasons that they don't understand, and follow it blindly. To make matters worse, Microsoft is carefully hiding the fact that those are non-portable functions. For example, if you look at the page for scanf_s,
    scanf_s, _scanf_s_l, wscanf_s, _wscanf_s_l
    There is absolutely NO mentioning that it is not a standard function, and can easily mislead, especially since it's part of <stdio.h>.

    Maybe we should start recommending newbies to start with GCC, and only (optionally) switch to MSVC when they are good enough to make their own judgments and not be misled by MS propagandas like this.

    It's soon going to be as bad as learning web designing with IE.

  14. #14
    Registered User
    Join Date
    May 2009
    Posts
    242
    interesting the discussion on deprecation and IDEs.

    to Salem: So, if the number is supposed to be the SIZE of the destination string buffer, and my destination string buffer is FullName[46], does that mean that I need to use 46 every time? like as first command

    strcpy_s(FullName, 46, Last);

  15. #15
    Registered User
    Join Date
    May 2009
    Posts
    242
    using 46 (size of FullName) the whole way through makes it work.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. No clue how to make a code to solve problems!
    By ctnzn in forum C Programming
    Replies: 8
    Last Post: 10-16-2008, 02:59 AM
  2. C Pointers Problems
    By mhelal in forum C Programming
    Replies: 8
    Last Post: 01-10-2007, 05:35 AM
  3. String Manipulation problems -_-
    By Astra in forum C Programming
    Replies: 5
    Last Post: 12-13-2006, 04:48 PM
  4. Rendering problems (DirectX?)
    By OnionKnight in forum Tech Board
    Replies: 0
    Last Post: 08-17-2006, 12:17 PM
  5. DJGPP problems
    By stormswift in forum C Programming
    Replies: 2
    Last Post: 02-26-2002, 03:35 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21