Passing a pointer to place in array

Question

Learning C and having some issues with pointers/arrays. Using MPLAB X and a PIC24FJ128GB204 as target device, but I don't really think it matters for this question. The answer might be obvious, but without much background in C (yet), it is difficult to find a similar question that I understand enough to draw conclusions.

I have written an I2C library with the following function:

int I2C1_Write(char DeviceAddress, unsigned char SubAddress, unsigned char *Payload, char ByteCnt){
    int PayloadByte = 0;
    int ReturnValue = 0;
    char SlaveWriteAddr;

    // send start
    if(I2C1_Start() < 0){
        I2C1_Bus_SetDirty;
        return I2C1_Err_CommunicationFail;
    }

    // address slave
    SlaveWriteAddr = (DeviceAddress << 1) & ~0x1;       // shift left, AND with 0xFE to keep bit0 clear
    ReturnValue = I2C1_WriteSingleByte(SlaveWriteAddr);
    switch (ReturnValue){
        case I2C1_OK:
            break;
        case I2C1_Err_NAK:
            I2C1_Stop();
            I2C1_Bus_SetDirty;
            return I2C1_Err_BadAddress;
        default:
            I2C1_Stop();
            I2C1_Bus_SetDirty;
            return I2C1_Err_CommunicationFail;

    }

    // part deleted for brevity

    // and finally payload
    for(PayloadByte = 0; PayloadByte < ByteCnt; PayloadByte++){
        // send byte one by one
        if(I2C1_WriteSingleByte(Payload[PayloadByte]) != I2C1_ACK){
            I2C1_Stop();
            I2C1_Bus_SetDirty;
            return I2C1_Err_CommunicationFail;            
        }
    }
    return I2C1_OK;
}   

I want to call this function from another one, using a predefined const:

const unsigned char CMD_SingleShot[3] = {2, 0x2C, 0x06};

This has the length of the command as first byte, then the command bytes.

The calling function is:

int SHT31_GetData(unsigned char MeasurementData[]){
    // address device and start measurement
    if(I2C1_Write(SHT31_Address,
                0xFF,
                CMD_SingleShot[1],       // this is where the error message is given
                CMD_SingleShot[0])
                < 1){
        return -1;
    }
    //code omitted for brevity

    return 1;
}

The error message:

../Sensirion_SHT31.c:40:17: warning: passing argument 3 of 'I2C1_Write' makes pointer from integer without a cast

../I2C1.h:66:5: note: expected 'unsigned char *' but argument is of type 'unsigned char'

The problem is clearly (unsigned char)CMD_SingleShot[1] where I try to give a pointer to the second byte of the unsigned char array.

I have tried:

• reading up on pointers and arrays and trying to understand
• searching for similar functions
• given up on understanding and trying random thing hoping the error messages would lead me to the correct way of doing this. Things like:

CMD_SingleShot[1]
&CMD_SingleShot[1]
(unsigned char)CMD_SingleShot + 1

this just gave other error messages.

My questions:

• given the I2C1_Write function as is (expecting a unsigned char *) (for instance, if that was not my code and I couldn't change that), how would I pass the pointer to the second byte in the cont unsigned char array? My understanding is that an array is a pointer, so
• since this is my code, is there a better way to do this altogether?

Answer 1

First, don't do casting unless you know better than the compiler what is going on. Which, in your case, you don't. This is nothing to be ashamed of.

Doing &CMD_SingleShot[1] is a step in the right direction. The problem is that CMD_SingleShot[1] is of type const unsigned char and therefore taking the address of that gives you a pointer of type const unsigned char *. This cannot be passed to the Payload parameter since it expects a pointer of unsigned char *. Luckily you don't modify whatever Payload points to, so there is no reason for that to be non-const. Just change the definition of Payload to const unsigned char * and the compiler should be happy.

And by the way, in c, &Foo[n] is the same as Foo + n. Whatever you write is a matter of taste.

Edit:

Sometimes you don't have access to the library source code, and because of bad library design you are forced to cast. In that case, it is up to you to get things right. The compiler will happily shoot you in the foot if you ask it to.

In your case, the correct cast would be (unsigned char *)&CMD_SingleShot[1] and NOT (unsigned char *)CMD_SingleShot[1]. The first case interprets a pointer of one type as a pointer of different type. The second case interprets an unsigned character as a pointer, which is very bad.

Answer 2

Passing the address of the second byte of your command is done with either

&CMD_SingleShot[1]

or

CMD_SingleShot+1

But then you will run into an invalid conversion error since your command is defined as const unsigned char and then &CMD_SingleShot[1] is of type const unsigned char* but your function expects unsigned char*.

What you can do is either change the argument of your function:

int I2C1_Write(char DeviceAddress, unsigned char SubAddress, const unsigned char *Payload, char ByteCnt)

or cast your passing argument:

I2C1_Write(SHT31_Address, 0xFF, (unsigned char*)&CMD_SingleShot[1], CMD_SingleShot[0])

In the latter case be aware that casting away const'ness might result in undefined behaviour when changing it afterwards.

Answer 3

The function call is mostly correct, but since the 3rd parameter of the function is a pointer, you must pass an address to an array accordingly, not a single character. Thus &CMD_SingleShot[1] rather than CMD_SingleShot[1].

if(I2C1_Write(SHT31_Address,
                0xFF,
                &CMD_SingleShot[1], 
                CMD_SingleShot[0])
                < 1)

However when you do this, you claim that you get "discards qualifiers from pointer target" which is a remark about const correctness - apparently CMD_SingleShot is const (since it's a flash variable or some such?).

That compiler error in turn simply means that the function is incorrectly designed - an I2C write function should clearly not modify the data, just send it. So the most correct fix is to change the function to take const unsigned char *Payload. Study const correctness - if a function does not modify data passed to it by a pointer, then that pointer should be declared as const type*, "pointer to read-only data of type".

If it wouldn't be possible to change the function because you are stuck with an API written by someone else, then you'd have to copy the data into a read/write buffer before passing it to the function. "Casting away" const is almost never correct (though most often works in practice, but without guarantees).

---

Other concerns:

• When programming C in general, and embedded C in particular, you should use stdint.h instead of the default types of C, that are problematic since they have variable sizes.

• Never use char (without unsigned) for anything else but actual strings. It has implementation-defined signedness and is generally dangerous - never use it for storing raw data.

• When programming an 8 bit MCU, always use uint8_t/int8_t when you know in advance that the variable won't hold any larger values than 8 bits. There are plenty of cases where the compiler simply can't optimize 16 bit values down to 8 bit.

• Never use signed or potentially signed operands to the bitwise operators. Code such as (DeviceAddress << 1) & ~0x1 is wildly dangerous. Not only is DeviceAddress potentially signed and could end up negative, it gets implicitly promoted to int. Similarly, 0x1 is of type int and so on 2's complement PIC ~0x1 actually boils down to -2 which is not what you want.

  Instead try to u suffix all integer constants and study Implicit type promotion rules.