Use strcpy rather than strdup #5610
Conversation
mapstring.c
Outdated
|
|
||
| pszReturn = strdup( pszString ); | ||
| pszReturn = msSmallMalloc(strlen(pszString) + 1); |
There was a problem hiding this comment.
a more efficient implementation would store strlen(pszString) in a nStringLength variable for reuse. And instead of strcpy(), you would do memcpy(pszReturn, pszString, nStringLength + 1 /* nul terminated byte*/)
|
@rouault - thanks for the review. |
mapstring.c
Outdated
|
|
||
| pszReturn = strdup( pszString ); | ||
| unsigned int nStringLength = strlen(pszString); |
There was a problem hiding this comment.
use size_t for type safety instead of unsigned int
mapstring.c
Outdated
| pszReturn = strdup( pszString ); | ||
| unsigned int nStringLength = strlen(pszString); | ||
| char *pszReturn = malloc(nStringLength + 1); | ||
| memcpy(pszReturn, pszString, nStringLength + 1); /* null terminated byte*/ |
There was a problem hiding this comment.
memcpy() should be done after the below NULL check
mapstring.c
Outdated
|
|
||
| pszReturn = strdup( pszString ); | ||
| unsigned int nStringLength = strlen(pszString); |
There was a problem hiding this comment.
Being in a .c file, the variable declarations should be done at the beginning of the function to please MSVC
size_t nStringLength;
char* pszReturn;
mapstring.c
Outdated
| { | ||
| char *pszReturn; | ||
| size_t nStringLength = strlen(pszString) + 1; /* null terminated byte*/ |
There was a problem hiding this comment.
the declaration and its affectation of both variables should be separated. Namely after the if (pszString == NULL) pszString = ""; check
You're close ;-)
mapstring.c
Outdated
| pszReturn = strdup( pszString ); | ||
| if (pszReturn == NULL) { | ||
| fprintf(stderr, "msSmallMalloc(): Out of memory allocating %ld bytes.\n", | ||
| (long)strlen(pszString)); |
There was a problem hiding this comment.
strlen(pszString) could be replaced by nStringLength
|
@rouault - thanks for your patience! Variables now set after the NULL check. |
I guess I must have missed this in the original pull request. This was a fix supplied by @tbonfort and relates to this - #5277
This pull request includes the code from https://github.com/tbonfort/mapserver/commit/bacf44f24a2dab8eff47d98788a302b7e0b0bb49
If strdup is used on Windows/MSVC then char variables don't get set correctly (at least when using the debugger).