Official "String copy Statement Considered Harmful" thread

[Dubslow]

Originally Posted by flashjh

Probably should have them already, do you have a link to mfaktc source files?

http://mersenneforum.org/mfaktc/ (The unqualified 0.18 would be the source.)

Originally Posted by flashjh

I'm pretty sure I need _strnicmp instead of strncasecomp and strcpy_s instead of strcpy,

I don't see anywhere in the mfatkc source about strcpy_s, but I'll keep looking, and you certainly know better than me about MSVC
Edit: http://msdn.microsoft.com/en-us/libr...(v=vs.80).aspx

Originally Posted by MS

For example, the strcpy function has no way of telling if the string that it is copying is too big for its destination buffer. However, its secure counterpart, strcpy_s, takes the size of the buffer as a parameter, so it can determine if a buffer overrun will occur. If you use strcpy_s to copy eleven characters into a ten-character buffer, that is an error on your part; strcpy_s cannot correct your mistake, but it can detect your error and inform you by invoking the invalid parameter handler.

strcpy_s has a different definition, requiring a third size argument; it's just meant to be a "catch stupid programmer error" thing. Since the code does a check for a long line anyways, the extra functionality is unnecessary.

bill@Gravemind:~/CUDALucas/test∰∂ cat parse.c
#include <stdio.h>
#include <string.h>
#include <limits.h>
#include <ctype.h>
#include <errno.h>
#include <stdlib.h>

#ifdef _MSC_VER
#define strncasecmp _strnicmp
#define _CRT_SECURE_NO_WARNINGS
#endif

int isprime(unsigned int n)
/*
returns
0 if n is composite
1 if n is prime
*/
{
  unsigned int i;
  
  if(n<=1) return 0;
  if(n>2 && n%2==0)return 0;

  i=3;
  while(i*i <= n && i < 0x10000)
  {
    if(n%i==0)return 0;
    i+=2;
  }
  return 1;
}

Originally Posted by flashjh

however, I don't know what to do with the 'conditional expression is constant'.

Oh, that's just a while(1), that warning can be ignored. (Perhaps deleting the 1 and leaving the conditional blank will work?) Edit: Actually, in further looking, I actually have no clue why that loop is there at all. If the func finds a too-long line, then it reads in the characters until a control character then breaks? Wtf?


PS I forgot to mention above, but the help message was moved from no-args to the -h arg.
PPS Here's a slightly larger .ini file:

# Polite is the same as the -polite option. If it's 1, each iteration is
# polite. If it's (for example) 12, then every 12th iteration is polite. Thus
# the higher the number, the less polite the program is. Set to 0 to turn off
# completely. Polite!=0 will incur a slight performance drop, but the screen 
# should be more responsive. Trade responsiveness for performance. (Note:
# polite=0 is known to cause CUDALucas to use some extra CPU time; Polite=64 or
# higher is a good compromise.)
Polite=1

flash, could you add this to your copy before you bundle the executable?

The attached archive contains the modified parse.c and CUDALucas.ini.

[LaurV]

Originally Posted by Dubslow

strcpy_s has a different definition, requiring a third size argument; it's just meant to be a "catch stupid programmer error" thing.

[offtopic]That is totally false. It is meant to catch malicious guys feeding you with a malicious string at run time. Have a look at buffer overflow thingies, and plenty of viruses/trojans who exploit it. Classical strcpy will copy a string till a \0 is found. If there is not one found, all your memory could be overwritten. As both the string (data) and the code are in the memory, if your program does not take special precautions, then I can make a malicious string that will be copied over (overwrite) part of your program. Please make a habit to use "safe" string functions every time you can (that should be 99.99%of the cases). If I know where your string is in memory (easy to find out, search the common buffers for ascii characters) then I can replace the \0 and you program goes in the woods.[/offtopic]

[Dubslow]

Originally Posted by LaurV

[offtopic]That is totally false. It is meant to catch malicious guys feeding you with a malicious string at run time. Have a look at buffer overflow thingies, and plenty of viruses/trojans who exploit it. Classical strcpy will copy a string till a \0 is found. If there is not one found, all your memory could be overwritten. As both the string (data) and the code are in the memory, if your program does not take special precautions, then I can make a malicious string that will be copied over (overwrite) part of your program. Please make a habit to use "safe" string functions every time you can (that should be 99.99%of the cases). If I know where your string is in memory (easy to find out, search the common buffers for ascii characters) then I can replace the \0 and you program goes in the woods.[/offtopic]

Ah, thanks for that clarification. Your choice, flash. Strings are 100 chars (plus null terminator) in parse.c.

[Dubslow]

Try this on for size:

bill@Gravemind:~/CUDALucas/test∰∂ cat parse.c

#include <stdio.h>
#include <string.h>
#include <limits.h>
#include <ctype.h>
#include <errno.h>
#include <stdlib.h>

#ifdef _MSC_VER
#define strncasecmp _strnicmp
#endif

void strcopy(char* dest, char* src, size_t n) 
{
#ifdef _MSC_VER
	strcpy_s(dest, n, src);
#else
	strncpy(dest, src, n);
#endif
}

int isprime(unsigned int n) //...etc

Turns out the Linux "safe" equivalent is strncpy, much like strncasecmp. It's too bad Microsoft can't just use the same damn function. (Note the different ordering of arguments, as well as the name.)

Where it gave warnings, replace those "strcpy" calls with "strcopy", and add a third argument "MAX_LINE_LENGTH+1". Alternately, just DL the attached modified version

[flashjh]

I'll work compiling tomorrow. I have to get some

[kjaget]

Originally Posted by Dubslow

Try this on for size:

bill@Gravemind:~/CUDALucas/test∰∂ cat parse.c

#include <stdio.h>
#include <string.h>
#include <limits.h>
#include <ctype.h>
#include <errno.h>
#include <stdlib.h>

#ifdef _MSC_VER
#define strncasecmp _strnicmp
#endif

void strcopy(char* dest, char* src, size_t n) 
{
#ifdef _MSC_VER
	strcpy_s(dest, n, src);
#else
	strncpy(dest, src, n);
#endif
}

int isprime(unsigned int n) //...etc

Turns out the Linux "safe" equivalent is strncpy, much like strncasecmp. It's too bad Microsoft can't just use the same damn function. (Note the different ordering of arguments, as well as the name.)

Where it gave warnings, replace those "strcpy" calls with "strcopy", and add a third argument "MAX_LINE_LENGTH+1". Alternately, just DL the attached modified version

strncpy() isn't a safe version of strcpy(). It doesn't report errors and will not nul-terminate a string if it hits the size limit. That means it itself won't cause buffer overruns, but any subsequent use of the results of likely will if your source string was too long.

[LaurV]

Originally Posted by kjaget

strncpy() isn't a safe version of strcpy().

True. strncpy() is not a "version" that strenghten the "safety" of strcpy. It is a DIFERENT function. And it is a SAFE function.

[Dubslow]

Originally Posted by kjaget

strncpy() isn't a safe version of strcpy(). It doesn't report errors and will not nul-terminate a string if it hits the size limit. That means it itself won't cause buffer overruns, but any subsequent use of the results of likely will if your source string was too long.

I see, thanks for pointing that out as well. (strcpy_s() does guarantee a null-terminated string.) However, strncpy is certainly safer than strcpy, and I don't know of any other alternatives on Linux. (And, as I said, the code in question does check for a long line anyways, so even if it's not terminated it would be caught.)

[chalsall]

Originally Posted by kjaget

strncpy() isn't a safe version of strcpy(). It doesn't report errors and will not nul-terminate a string if it hits the size limit. That means it itself won't cause buffer overruns, but any subsequent use of the results of likely will if your source string was too long.

strncpy(dest, src, length);
dest[length-1]=0;

...is pretty safe....

[Dubslow]

Originally Posted by chalsall

strncpy(dest, src, length);
dest[length-1]=0;

...is pretty safe....

But you don't know for sure without further code if some part of src got clobbered. strcpy_s throws an error if dest is too small for src.

Originally Posted by MSFT

If strDestination or strSource is a null pointer, or if the destination string is too small, the invalid parameter handler is invoked as described in Parameter Validation. If execution is allowed to continue, these functions return EINVAL and set errno to EINVAL.

Upon successful execution, the destination string will always be null terminated.

[chalsall]

Originally Posted by Dubslow

But you don't know for sure without further code if some part of src got clobbered.

src wouldn't get clobbered; dest simply wouldn't have the full string.

int strcpy_s(
   char *dest,
   int n,
   const char *src
)
{
   if (!dest || !src) {
      return 1;   // Stupid human...
   }

   if (strlen(src) > n-1) {
      return 2;   // dest too small;  Stupid human...
   }

   strncpy(dest, src, n);

   return 0;
}