0

I have a requirement where I have to get the RFID RSSI value which is an int and convert it to a char pointer and append to it. Below is how I did it.

char *epcBytes = (char *)tag_operation_report->tag.epc.bytes;

        int rssiString = fabs(tag_operation_report->tag.rssi);

        char *rssiVal = (char *)rssiString;

        char* rssi = "rssi";

        char *rssiResult = malloc(strlen(&rssi) + strlen(&rssiVal) + 1);
        strcpy(rssiResult, &rssi);
        strcpy(rssiResult + strlen(&rssi), &rssiVal);

        char *result = malloc(strlen(&epcBytes) + strlen(&rssiResult) + 1);
        strcpy(result, &epcBytes);
        strcpy(result + strlen(&epcBytes), &rssiResult);
        data = (void*)result;

But I am getting the following exception while I run the code.

Unhandled exception at 0x00007FFBD017B2E5 (msvcr120d.dll) in RFIDTest.exe: 0xC0000005: Access violation writing location 0x0000000000000000.

What am I doing wrong here? When I run this on an online C compiler, it runs fine. This exception is thrown at the line,

strcpy(rssiResult, &rssi);
AnOldSoul
  • 4,017
  • 12
  • 57
  • 118
  • 1
    Are you trying to convert an `int` to a string by simply casting it to `char *`? – ForceBru Jan 18 '16 at 11:30
  • `Unhandled exception at 0x00007FFBD017B2E5 (msvcr120d.dll)` suggests that you are using a 64-bit Windows. On 64-bit Windows, pointers are 64-bit and `int` is 32-bit, so you have a problem here. `intptr_t` is the appropriate integer to store pointers. You probably don't mean to do that anyway. – ElderBug Jan 18 '16 at 11:34
  • In addition to the numerous bugs mentioned in the answer, this is needlessly inefficient: `char* rssi = "rssi"; ... malloc(strlen(&rssi)`. Instead do `const char rssi [] = "rssi"; ... malloc(sizeof(rssi)-1 + ...` – Lundin Jan 18 '16 at 12:18

1 Answers1

2
  1. You see, in char * is not a string type it's just a pointer. So this

    char *rssiVal = (char *)rssiString;
    

    is completely wrong, you are casting an integer to a pointer. This is defined by the standard but it will in no way do what you are trying, to convert an integer to a string you need something like snprintf(), this is an example

    char string[100];
    int result; 
    result = snprintf(string, sizeof(string), 
        "%.0f", fabs(tag_operation_report->tag.rssi));
    if ((result < 0) || (result >= sizeof(string))
        error_please_dont_use_string();
    

    also, fabs() returns double not int so you risk an overflow if you assign the value to an int.

  2. You are passing a char ** pointer to strlen() here

    char *rssiResult = malloc(strlen(&rssi) + strlen(&rssiVal) + 1);
    /*                               ^ wrong --------^ remove them */
    

    while strlen() is expecting a char * pointer to a nul terminated sequence of bytes. Doing this invokes undefined behavior.

  3. And you do it again here

    strcpy(rssiResult, &rssi);
    /*                 ^ same problem, remove it */
    

    The difference is that rssi + 1 is not the same as &rssi + 1 so when the pointer is dereferenced in either of these functions undefined behavior occurs.

  4. I am very surprised to see code like this since the compiler should warn about incompatible pointer types, there are two possible reasons to see this code and they are, either you don't compile the code with warning flags like -Wall for gcc or you completely ignore warnings.

  5. Also, always check that malloc() succeeded before using the pointer.

Note: don't use strlen() as if the length was stored somewhere, every time you call strlen() the length needs to be computed, store the length somewhere instead. That gives you another advantage, instead of using strcpy() which will again search for the nul terminator byte since you have the length stored somewhere you can use memcpy(). You might not notice any performance improvement, but at least you would know that you took advantage of how things work to achieve better performance.

Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97
  • Im getting an another access violation writing to location error at this line. strcpy(rssiResult, rssi); Can't rssiResult be a char pointer? – AnOldSoul Jan 19 '16 at 04:05