Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #70665 > unrolled thread

Question about type casting

Back to article view | Back to comp.lang.c

Contents

  Question about type casting fl <rxjwg98@gmail.com> - 2015-09-16 10:52 -0700
    Re: Question about type casting Ben Bacarisse <ben.usenet@bsb.me.uk> - 2015-09-16 20:01 +0100
      Re: Question about type casting David Brown <david.brown@hesbynett.no> - 2015-09-17 09:02 +0200
        Re: Question about type casting supercat@casperkitty.com - 2015-09-17 12:59 -0700

#70665 — Question about type casting

Hi,

I read an example .c embedded firmware project, which has a device driver
 level library. Here is the board level .c file, which has some board specific
 variable definitions:

This line is from a header file:
typedef uint32_t GPIO_PinConfig;

Below is from the board level .c file:
GPIO_PinConfig gpioPinConfigs[] = {
    /* Input pins */
    /* EK_TM4C1294XL_USR_SW1 */
    GPIOTiva_PJ_0 | GPIO_CFG_IN_PU | GPIO_CFG_IN_INT_RISING,
    /* EK_TM4C1294XL_USR_SW2 */
    GPIOTiva_PJ_1 | GPIO_CFG_IN_PU | GPIO_CFG_IN_INT_RISING,

    /* Output pins */
    /* EK_TM4C1294XL_USR_D1 */
    GPIOTiva_PN_1 | GPIO_CFG_OUT_STD | GPIO_CFG_OUT_STR_HIGH,
    /* EK_TM4C1294XL_USR_D2 */
    GPIOTiva_PN_0 | GPIO_CFG_OUT_STD | GPIO_CFG_OUT_STR_HIGH
};

/* The device-specific GPIO_config structure */
const GPIOTiva_Config GPIOTiva_config = {
    .pinConfigs = (GPIO_PinConfig *) gpioPinConfigs,
    .callbacks = (GPIO_CallbackFxn *) gpioCallbackFunctions,
    .numberOfPinConfigs = sizeof(gpioPinConfigs) / sizeof(GPIO_PinConfig),
    .numberOfCallbacks = sizeof(gpioCallbackFunc)/sizeof(GPIO_CallbackFxn),
    .intPriority = (~0)
};

Here is from the library definition (.c file):
/* Device specific interpretation of the GPIO_PinConfig content */
typedef struct PinConfig {
    uint8_t pin;
    uint8_t port;
    uint16_t config;
} PinConfig;

void GPIO_clearInt(unsigned int index)
{
    PinConfig *config = (PinConfig *) &GPIOTiva_config.pinConfigs[index];
...
}

I have a question about the casting of (PinConfig *) from the last line.
'GPIOTiva_config.pinConfigs[index]' has been initialized with
 gpioPinConfigs, which is in fact a 4 unsigned int array. Now, in the last
 line, it is cast to a PinConfig, which has two uint_8t one uint16_t
 struct, pointer. '4 unsigned int' is obvious different from struct
 PinConfig.

Could you help me understanding this casting?

Thanks,

[toc] | [next] | [standalone]

#70678

fl <rxjwg98@gmail.com> writes:

> I read an example .c embedded firmware project, which has a device driver
>  level library. Here is the board level .c file, which has some board specific
>  variable definitions:
>
> This line is from a header file:
> typedef uint32_t GPIO_PinConfig;
>
> Below is from the board level .c file:
> GPIO_PinConfig gpioPinConfigs[] = {
>     /* Input pins */
>     /* EK_TM4C1294XL_USR_SW1 */
>     GPIOTiva_PJ_0 | GPIO_CFG_IN_PU | GPIO_CFG_IN_INT_RISING,
>     /* EK_TM4C1294XL_USR_SW2 */
>     GPIOTiva_PJ_1 | GPIO_CFG_IN_PU | GPIO_CFG_IN_INT_RISING,
>
>     /* Output pins */
>     /* EK_TM4C1294XL_USR_D1 */
>     GPIOTiva_PN_1 | GPIO_CFG_OUT_STD | GPIO_CFG_OUT_STR_HIGH,
>     /* EK_TM4C1294XL_USR_D2 */
>     GPIOTiva_PN_0 | GPIO_CFG_OUT_STD | GPIO_CFG_OUT_STR_HIGH
> };
>
> /* The device-specific GPIO_config structure */
> const GPIOTiva_Config GPIOTiva_config = {
>     .pinConfigs = (GPIO_PinConfig *) gpioPinConfigs,
>     .callbacks = (GPIO_CallbackFxn *) gpioCallbackFunctions,
>     .numberOfPinConfigs = sizeof(gpioPinConfigs) / sizeof(GPIO_PinConfig),
>     .numberOfCallbacks = sizeof(gpioCallbackFunc)/sizeof(GPIO_CallbackFxn),
>     .intPriority = (~0)
> };
>
> Here is from the library definition (.c file):
> /* Device specific interpretation of the GPIO_PinConfig content */
> typedef struct PinConfig {
>     uint8_t pin;
>     uint8_t port;
>     uint16_t config;
> } PinConfig;
>
> void GPIO_clearInt(unsigned int index)
> {
>     PinConfig *config = (PinConfig *) &GPIOTiva_config.pinConfigs[index];
> ...
> }
>
> I have a question about the casting of (PinConfig *) from the last line.
> 'GPIOTiva_config.pinConfigs[index]' has been initialized with
>  gpioPinConfigs, which is in fact a 4 unsigned int array. Now, in the last
>  line, it is cast to a PinConfig, which has two uint_8t one uint16_t
>  struct, pointer. '4 unsigned int' is obvious different from struct
>  PinConfig.
>
> Could you help me understanding this casting?

The author(s) want(s) to have have two different views of the same data.
For some purposes they want to view the config as an uint23_t, probably
to make the setting using an "or" of some constants easy, but at other
times they want to view it as two bytes and a short integer.

There are all sorts of pitfalls doing this, but since the code is very
system-specific, no doubt they've checked that it will all work with the
compiler they are using.

It's usually better to do this using a union because the cast is a lie
since the data in question are really uint32_t objects not PinConfig
objects.

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#70774

On 16/09/15 21:01, Ben Bacarisse wrote:
> fl <rxjwg98@gmail.com> writes:
> 
>> I read an example .c embedded firmware project, which has a device driver
>>  level library. Here is the board level .c file, which has some board specific
>>  variable definitions:
>>
>> This line is from a header file:
>> typedef uint32_t GPIO_PinConfig;
>>
>> Below is from the board level .c file:
>> GPIO_PinConfig gpioPinConfigs[] = {
>>     /* Input pins */
>>     /* EK_TM4C1294XL_USR_SW1 */
>>     GPIOTiva_PJ_0 | GPIO_CFG_IN_PU | GPIO_CFG_IN_INT_RISING,
>>     /* EK_TM4C1294XL_USR_SW2 */
>>     GPIOTiva_PJ_1 | GPIO_CFG_IN_PU | GPIO_CFG_IN_INT_RISING,
>>
>>     /* Output pins */
>>     /* EK_TM4C1294XL_USR_D1 */
>>     GPIOTiva_PN_1 | GPIO_CFG_OUT_STD | GPIO_CFG_OUT_STR_HIGH,
>>     /* EK_TM4C1294XL_USR_D2 */
>>     GPIOTiva_PN_0 | GPIO_CFG_OUT_STD | GPIO_CFG_OUT_STR_HIGH
>> };
>>
>> /* The device-specific GPIO_config structure */
>> const GPIOTiva_Config GPIOTiva_config = {
>>     .pinConfigs = (GPIO_PinConfig *) gpioPinConfigs,
>>     .callbacks = (GPIO_CallbackFxn *) gpioCallbackFunctions,
>>     .numberOfPinConfigs = sizeof(gpioPinConfigs) / sizeof(GPIO_PinConfig),
>>     .numberOfCallbacks = sizeof(gpioCallbackFunc)/sizeof(GPIO_CallbackFxn),
>>     .intPriority = (~0)
>> };
>>
>> Here is from the library definition (.c file):
>> /* Device specific interpretation of the GPIO_PinConfig content */
>> typedef struct PinConfig {
>>     uint8_t pin;
>>     uint8_t port;
>>     uint16_t config;
>> } PinConfig;
>>
>> void GPIO_clearInt(unsigned int index)
>> {
>>     PinConfig *config = (PinConfig *) &GPIOTiva_config.pinConfigs[index];
>> ...
>> }
>>
>> I have a question about the casting of (PinConfig *) from the last line.
>> 'GPIOTiva_config.pinConfigs[index]' has been initialized with
>>  gpioPinConfigs, which is in fact a 4 unsigned int array. Now, in the last
>>  line, it is cast to a PinConfig, which has two uint_8t one uint16_t
>>  struct, pointer. '4 unsigned int' is obvious different from struct
>>  PinConfig.
>>
>> Could you help me understanding this casting?
> 
> The author(s) want(s) to have have two different views of the same data.
> For some purposes they want to view the config as an uint23_t, probably
> to make the setting using an "or" of some constants easy, but at other
> times they want to view it as two bytes and a short integer.
> 
> There are all sorts of pitfalls doing this, but since the code is very
> system-specific, no doubt they've checked that it will all work with the
> compiler they are using.
> 
> It's usually better to do this using a union because the cast is a lie
> since the data in question are really uint32_t objects not PinConfig
> objects.
> 

I much prefer unions for this sort of thing:

typedef union {
	uint32_t raw;
	struct {
		uint8_t pin;
		uint8_t port;
		uint16_t config;
	};
} PinConfig;

Many manufacturers use that sort of structure in their headers
describing their hardware.

Unfortunately, most hardware manufacturers (TI in this case, I believe)
write absolute crap in the sample code, "drivers", "abstraction layers",
"code wizards", and so on.  It regularly looks like it's done by interns
or student projects with no guidance or control, and certainly no effort
to ensure that the code is accurate.  It is very common to find that the
code works when you are doing "debug" builds (their tools always like to
make a distinction between "debug" builds at -O0 -g and "release" builds
at -Os or -O2 - a terrible idea, IMHO), but fails when compiling with
optimisation.  Or maybe they work on -Os, but no one has ever thought to
check -O3 - which then fails, because the code is not correct C but just
happens to work if the compiler doesn't try too hard.

Add to that that configuration code like this stuff is a complicated
mess and much harder to understand than just reading the devices
reference manual, and typically vastly inefficient (needing several
layers of function calls to do something that is actually just writing a
single value to a single address).

Usually I write my own code for such things, using just the base header
file that defines the structures and bitmasks for the chip and peripherals.

[toc] | [prev] | [next] | [standalone]

#70848

On Thursday, September 17, 2015 at 2:02:17 AM UTC-5, David Brown wrote:
> Unfortunately, most hardware manufacturers (TI in this case, I believe)
> write absolute crap in the sample code, "drivers", "abstraction layers",
> "code wizards", and so on.

I wonder how many surveys have been done about the extent to which
programmers use such things as-is and somehow don't hit the bugs therein,
use them as-is but work around the bugs, patch them to fix the bugs, use
them for basic device setup and then ignore them, or simply ignore basically
everything that uses identifiers not found in the hardware data sheet?

To my mind, 

    atomicRMW32(&GTMR3->MODE, TMR_MODE_PRESCALE_MASK, TMR_MODE_PRESCALE_DIV_4);

is better in basically every way than 

    SetTimerMode(&timer3, TMR_POS_EDGE, 4);

since someone can search the data sheet for "GTMR", find the chapter
describing it, go to the register definitions, search for "MODE", and find
out what the mode bits do, and infer from that what's going on.  Otherwise,
one would have to look at both the documentation for the API function and
then *still* have to look at the data sheet to see whether there are any
interactions among the mode settings.  Further, if the hardware-defined
mode choice are selected by values:

    0=TMR_MODE_PRESCALE_DIV_1
    1=TMR_MODE_PRESCALE_DIV_2
    2=TMR_MODE_PRESCALE_DIV_4
    3=TMR_MODE_PRESCALE_DIV_8
    4=TMR_MODE_PRESCALE_DIV_16
    5=TMR_MODE_PRESCALE_DIV_32
    6=TMR_MODE_PRESCALE_DIV_64
    7=TMR_MODE_PRESCALE_DIV_128

does a value of 4 passed to the API function indicate division by four or
does it indicate hardware option #4, i.e. division by 16?

>                             It is very common to find that the
> code works when you are doing "debug" builds (their tools always like to
> make a distinction between "debug" builds at -O0 -g and "release" builds
> at -Os or -O2 - a terrible idea, IMHO), but fails when compiling with
> optimisation.  Or maybe they work on -Os, but no one has ever thought to
> check -O3 - which then fails, because the code is not correct C but just
> happens to work if the compiler doesn't try too hard.

Atmel Studio bundles a version of GCC which doesn't always seem to pay
attention to "volatile" qualifers on bitfields used for registers.  I think
the Standard allows compilers to perform bitfield accesses using any sequence
of fresh or cached reads and writes they see fit, and I don't think that a
"volatile" qualifier is required to change that.  On the other hand, if code
like:

    while (!peripheral->SOMEREG.bits.READY && !--ctr) {}

is liable to get replaced with:

    if (!peripheral->SOMEREG.bits.READY) ctr = 0;

that pretty much negates any advantages it might have had over:

    while (!(peripheral->SOMEREG.reg & PERIPH_SOMEREG_READY) && !--ctr) {}

though of course the latter would be nicer if the silly ".reg" weren't
necessary.

> Add to that that configuration code like this stuff is a complicated
> mess and much harder to understand than just reading the devices
> reference manual, and typically vastly inefficient (needing several
> layers of function calls to do something that is actually just writing a
> single value to a single address).

It's pretty amazing how much of the stuff can be not only needlessly complex
but also wrong.  For example, one company's library includes a pair of
"disable interrupt" and "restore interrupt" functions which work by updating
a global counter when they're called, so that "restore interrupt" will only
re-enable interrupts when there have been enough calls to balance out all
the calls to "disable interrupt".  Such an approach is needlessly complex,
and will fail unless all code which enables or disables interrupts uses the
same library.

My own approach is simpler and easier: an inline function which will load
a register with the present enable/disable state, disable interrupts, and
return the register, and another inline function which will enable interrupts
when given a non-zero value.  It's possible to have an arbitrary number of
methods use the latter approach without any coordination, and provided that
code which enters via the "save interrupt state and disable interrupts"
always exits via the matching "restore interrupts" call, correct operation
will be assured even if none of the methods involved know anything about
each other.

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c

csiph-web