Why is memory access exception caused in this occasion?

Question

I have the following code where I need to format the output. But when I do so, a memory exception caused saying invalid memory access exception. What am I doing wrong here? Please advice.

char* strData = malloc(tag_operation_report->tag.epc.size / 2), 
char* strTemp= malloc(tag_operation_report->tag.epc.size / 2);
    do
    {
        for (int i = 0; i < tag_operation_report->tag.epc.size / 2; i++)
        {
            uint32_t num = tag_operation_report->tag.epc.bytes[i];
            strTemp=sprintf(L"%x ", num);
            strData+=strTemp;
        }

    } while (tag_operation_report->tag.epc.size != 0);
    data = (void *)strData;
    free(strTemp);
    free(strData);

Shown below is the exception.

Unhandled exception at 0x00007FF83C3C3A5D (msvcr120d.dll) in RFIDTest.exe: 0xC0000005: Access violation reading location 0x00000000000000E2.

R Sahu · Accepted Answer · 2015-11-19T05:53:39.767

Problem 1

strTemp=sprintf(L"%x ", num);

You are assigning an int to strTemp, which is a char*. I am surprised your compiler didn't warn you. It's not clear what your objective here is.

Problem 2

strData+=strTemp;

Once again, your compiler should have told you that this is an error. You cannot increment a char* by another char*.

Problem 3

free(strTemp);
free(strData);

You are calling free on pointer values that are different from the values returned by malloc. That causes undefined behavior.

Suggested Solution

Perhaps this is what you intended to do:

// Don't need to use dynamic memory allocation for this.
// It's only going to be used in sprintf. 20 characters should
// be large enough for that.
char strTemp[20];

// Bootstrap strData.
int strDataSize = 0;

char* strData = malloc(1); 
// If memory allocation failed, deal with the problem
if ( strData == NULL )
{
   exit(EXIT_FAILURE);
}

strData[0] = '\0';

for (int i = 0; i < tag_operation_report->tag.epc.size / 2; i++)
{
   uint32_t num = tag_operation_report->tag.epc.bytes[i];
   sprintf(strTemp, "%x ", num);

   // Increase the size of strData
   strDataSize += strlen(strTemp);

   // Allocate more memory for strData.
   strData = realloc(strData, strDataSize + 1);

   // If memory allocation failed, deal with the problem
   if ( strData == NULL )
   {
      exit(EXIT_FAILURE);
   }

   // Add strTemp to strData.
   strcat(strData, strTemp);
}

data = (void *)strData;

// Don't free strData. If you do, data will be dangling pointer.
// free(strData);

The objective is pretty obvious with Problem1. He thinks sprintf returns the formatted string, instead of taking it as the first argument. This mistake creates a ton of other problems (writing to the string-constant format string, and casting `uint32_t num` to `const char* format`). — Peter Cordes, Nov 19 '15 at 04:43
@PeterCordes, after going through the posted again, your comment makes sense. — R Sahu, Nov 19 '15 at 05:03

M.M · Answer 2 · 2015-11-19T04:12:28.437

This is a mistake: sprintf(L"%x ", num);

The first argument to sprintf is an allocated buffer into which the output will be placed. The second argument is the format string.

So you are trying to overwrite the string literal L"%x", and also num is not a valid format string. Your compiler should have warned you about this mistake - pay attention to the compiler output.

Doing strTemp = is also a mistake, as is strTemp += strData. You cannot add pointers and the compiler should have warned about this too.

Another mistake is data = (void *)strData; free(strData); , this leaves data pointing to freed memory.

If you are trying to concatenate strings you are going about it completely the wrong way. You need to understand that pointers are not strings. Pointers point to memory areas which may contain strings. You have to think about which blocks of memory you have allocated and where your pointers are pointing.

How can I get each value from the for loop and concatenate them? Please advice. — AnOldSoul, Nov 19 '15 at 04:12
@mayooran [see this thread](http://stackoverflow.com/questions/8465006/how-to-concatenate-2-strings-in-c) for some ideas. You must allocate the required amount of memory yourself. There is no magic. — M.M, Nov 19 '15 at 04:13

score 1 · Answer 3 · answered Nov 19 '15 at 04:10

sprintf only modifies already allocated memory. You have to pass the destination buffer pointer as first parameter. http://man7.org/linux/man-pages/man3/sprintf.3.html

If you aren't 100% sure that output will fit to your buffer it is better to use snprintf that takes one more parameter to tell how much space is left in the buffer. That prevents bugs caused by sprintf writing past the end of buffer.

Why is memory access exception caused in this occasion?

3 Answers3