c pointer decrement safe/unsafe?

Question

analyzing some code:

static volatile UCHAR *pucSndBufferCur;

eMBErrorCode eMBRTUSend( UCHAR ucSlaveAddress, const UCHAR * pucFrame, USHORT usLength )
{
    if( eRcvState == STATE_RX_IDLE )
    {
        /* First byte before the Modbus-PDU is the slave address. */
        pucSndBufferCur = ( UCHAR * ) pucFrame - 1;

        /* Now copy the Modbus-PDU into the Modbus-Serial-Line-PDU. */
        pucSndBufferCur[0] = ucSlaveAddress;

It looks like unsafe, what if memory in address (pucFrame - 1 ) is already used for other variable and overwriting it could cause malfunctions.

What do you think, is code like this can be used or its wrong way and never should be used ?

Althoug it might be unsafe when not proerly checked, it still dependent on the context. which means that under certain conditions it is not neccearily bad - However when writing your own code it is best to avoid these assumptions e.g. `pucFrame - 1` and It is better to use proper struct castings and other methods to avoid errors. but again these constructs are wildly used and don't neccesarily wrong... — Omer Dagan, Feb 19 '18 at 11:15
What is the structure definition that supports "First byte before the Modbus-PDU is the slave address"? — chux - Reinstate Monica, Feb 19 '18 at 12:21

score 7 · Accepted Answer · answered Feb 19 '18 at 11:43

From this comment

    /* First byte before the Modbus-PDU is the slave address. */
    pucSndBufferCur = ( UCHAR * ) pucFrame - 1;

we might infer that the function expects the caller to guarantee this. In that case, if the caller fulfils the requirement, then the code is safe.

If the caller doesn't guarantee this, the function is not safe. This is just like any other pre-requisite you can't usefully check: they have UB if the requirement is violated and this is the caller's fault.

For comparison, free() will likely have similar behaviour (and probably worse side-effects) if you violate the pre-requisite that its pointer argument was returned from malloc()/realloc().

It looks like unsafe, what if memory in address (pucFrame - 1 ) is already used for other variable and overwriting it could cause malfunctions

It is unsafe - it's C. The responsibility for using it correctly is yours. If you use it wrongly (passing a pointer that does not meet the stated requirement), there will be malfunctions and they will be your fault.

What do you think, is code like this can be used or its wrong way and never should be used ?

It can be used safely, just as free() can be used safely. It can be used the wrong way just as free() can be misused. The only reason to never use code like this is that you don't trust yourself to use it correctly.

The practical advice is to look at where this pucFrame pointer comes from, verify that it is always guaranteed to meet the requirement, and then make sure you don't break anything in the pointer's journey through your code.

Bathsheba · Answer 2 · 2018-02-19T11:17:15.410

1

The behaviour of the statement pucSndBufferCur = ( UCHAR * ) pucFrame - 1; will be undefined unless pucFrame is a pointer to an element within an array, or just after the end of the array, and there is one element before it. For this purpose a scalar can be considered as as an array of length one.

This is because pointer arithmetic is only valid within arrays.

You are also casting away const, which can yet again put the program into an undefined state, depending on what you do with the pointer (and the rules for this differ between C and C++).

edited Feb 19 '18 at 11:17

answered Feb 19 '18 at 11:06

Bathsheba

231,907
34
361
483

The behavior will also be defined if the C implementation defines it. As [IUseless](https://stackoverflow.com/users/212858/useless) notes, the comment about the slave address suggests there is implementation-defined behavior at work. – Eric Postpischil Feb 19 '18 at 12:47

score 1 · Answer 3 · answered Feb 19 '18 at 13:04

Never assume that something lies before or after a particular address unless you're checking for something within array bounds.

MISRA-C:2004, Rule 17.4 (Required) or MISRA-C:2012, Rule 18.4 (Required) Array indexing shall be the only allowed form of pointer arithmetic.

It is highly advised you use a static analyser like PC-Lint to check the sanity of your code to avoid undefined behaviours.

Also, you could put all the required things into a struct and just pass a struct pointer to the function. But even then, don't access struct members using ++ or -- , as you will have no idea about the padding added which is compiler dependent

score 0 · Answer 4 · answered Feb 19 '18 at 11:11

pls clear the sense of your question. Anyway code is a bit unsecure but NOT for the reason You tell. To make it more robust you should test against null. Pointer arithmetic is a bit dangerous, but I think this OLD code will make some assumptions based on context.

it seems to arrive from:

So yes, code from 2006 can be dangerous, but on the other side it has been used for 15 years... so it can be used. (old code usually works but if you do not modify it..)

c pointer decrement safe/unsafe?

4 Answers4