-1

I'm having some trouble using memcpy in that when the memcpy operation is performed I get:

"ÍÍWF03-021913.datýýýý««««««««þ"

when I should get:

"WF03-021913.datýýýý««««««««þ"

I don't know where these leading "ÍÍ" are coming from.

Code:

note: lpszFileName = "WF03-021913.dat"

typedef struct {
    BYTE  cbRequestType;
    BYTE  cbFileName;   
    char* szFileName;      
} UFTP_GET_FILE_INFO_REQUEST;

BOOL Uftp_BuildFileInfoRequest(PUFTP_REQUEST request, LPCTSTR lpszFileName)
{
    UFTP_GET_FILE_INFO_REQUEST *fileInfo;
    int fileNameLen;

    if (lpszFileName == NULL) {
        ASSERT( 0 );
        return FALSE;
    }

    fileNameLen = strlen( lpszFileName );
    if (fileNameLen == 0)
        return FALSE;

    request->dwRequestSize = sizeof(UFTP_GET_FILE_INFO_REQUEST) - 
                             sizeof(void*) + fileNameLen;
    request->RequestBuffer = malloc( request->dwRequestSize );
    if ( !request->RequestBuffer ) {
        TRACE0("Failed to allocate RequestBuffer");
        return FALSE;
    }

    fileInfo = (UFTP_GET_FILE_INFO_REQUEST*) request->RequestBuffer;
    fileInfo->cbRequestType = UFTP_GET_FILE_INFO;
    fileInfo->cbFileName = fileNameLen;

    memcpy(&fileInfo->szFileName, lpszFileName, fileNameLen);
    return TRUE;
}
Lou_257
  • 331
  • 1
  • 2
  • 7
  • Did you try `memcpy(&(fileInfo->szFileName), lpszFileName, fileNameLen);` I'm not sure about the operator order. – rekire Apr 12 '13 at 18:14
  • 2
    Can you show us `UFTP_GET_FILE_INFO_REQUEST`? – NPE Apr 12 '13 at 18:14
  • Why do you use `memcpy` instead of `strcpy`? – Some programmer dude Apr 12 '13 at 18:16
  • Also, can you print out `lpszFileName` right before the `memcpy()` and include that in your question. – NPE Apr 12 '13 at 18:16
  • If `szFileName` means null-terminated string, then you don't want the `&` in `memcpy(&fileInfo->szFileName, ...)`. – Jonathan Leffler Apr 12 '13 at 18:23
  • I added UFPT_GET_FILE_INFO_REQUEST and the string in lpszFileName. I've tried strcpy with the same result. memcpy(&(fileInfo-szFileName), lpszFileName, fileNameLen); gave the same result. – Lou_257 Apr 12 '13 at 18:27
  • @user2242764 just out of curiosity, where does this structure *go* once you're done properly building it? it it send over a wire or written to a file or some such? – WhozCraig Apr 12 '13 at 18:35
  • @WhozCraig it'll get sent as the buffer in a WinUSB_ControlTransfer command. – Lou_257 Apr 12 '13 at 18:37
  • OK, going to a USB device I assume you control etc. That being the case, you most likely want it represented a little differently Your *real* data format is a 2-byte preamble followed by a measured number of single-byte-chars by the looks of it (the measure being the second byte value) correct? also, if you do end up using a structure, it better be packed. – WhozCraig Apr 12 '13 at 18:48

1 Answers1

1

I'm only guessing here, but my guess is that fileInfo->szFileName is a pointer. This means that &fileInfo->szFileName is a pointer to a pointer, so you copy to a complete other area of memory.

Also, you don't copy the terminating '\0' character needed. You need fileNameLen + 1 for that, both when allocating and when copying.

If you really want it all in contiguous memory, you should probably change the structure to end with a character-array of size zero (may not be supported by your compiler, then use an array of size 1) and use sizeof(UFTP_GET_FILE_INFO_REQUEST) + fileNameLen + 1 as the size to allocate. Then you can use the array as a normal string array.


And if you fix those problems, you have yet another problem: You don't initialize the pointer to point to allocated memory. This means it will point to some random memory.

All of these errors will lead to undefined behavior, and I would say you are lucky it didn't crash.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • 1
    The cbFileName member holds the length, thus the terminating null is likely not required. If he copied fileNameLen+1 he would be exceeding the amount of memory he allocated. (see the malloc call). I concur with the rest of this answer, however, unless szFileName is a flexible array member (which we don't know without seeing it). – WhozCraig Apr 12 '13 at 18:22
  • @WhozCraig Unless the OP really want the strange characters after what looks like a proper filename, I think he/she uses it as a normal zero-terminated string. – Some programmer dude Apr 12 '13 at 18:25
  • 1
    That may well be what he's representing, but if it were required as part of the struct, there would be no need for the cbFileName member. If it *is* supposed to be a null-terminated string You can add an undersized malloc to your litany of things wrong with this. And judging by the structure presented in the updated code, it is trying desperately to be a flexible array member structure, but is failing miserably. – WhozCraig Apr 12 '13 at 18:33
  • +1 for the instructions on setting up the pseudo flexible array member. I think it is what he wants, but the structure should also be 1-byte packed. Also, if he includes the array[1] that extra byte in the def could actually count for the terminator, so he would be good with just `+ fileNameLen` – WhozCraig Apr 12 '13 at 18:51
  • yea pointer to a pointer was the issue, thanks for the very quick help. – Lou_257 Apr 12 '13 at 19:11