[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [avr-libc-dev] New pgm_read_ptr() macro?

From: David Brown
Subject: Re: [avr-libc-dev] New pgm_read_ptr() macro?
Date: Tue, 01 Jun 2010 14:40:23 +0200
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20100317 Lightning/1.0b1 Thunderbird/3.0.4

On 01/06/2010 13:34, Dean Camera wrote:

I've been pondering the addition of a new "pgm_read_ptr()" macro
to<avr/pgmspace.h>  to fill a gap (in my eyes) in the current API.
Currently, to read out pointers stored in PROGMEM space, one has to
use pgm_read_word() and cast the result to the desired pointer. This
is bad (IMHO) as it requires specific knowledge of the exact pointer
width on the AVRs, which could in theory change, and it makes the
code a little harder to read.

I'm proposing a simple extra macro be added:

#define pgm_read_ptr( x )    (void*)pgm_read_word( x )

Which would hide this from the user, so they do not have to use the
more ambiguous macros when dealing with progmem pointers. This would
also be helpful as it is implicitly typecast to the correct pointer
type when used as a initialiser, like:

char* FlashPtr = pgm_read_ptr(&FlashPointer);

Without user intervention. Thoughts anyone?

Cheers! - Dean

How about a more general solution based on typeof and sizeof?

#define pgm_read(v) ((typeof(*v)) \
        (__builtin_choose_expr(sizeof(*v) == 1, pgm_read_byte(v), \
        __builtin_choose_expr(sizeof(*v) == 2, pgm_read_word(v), \
        __builtin_choose_expr(sizeof(*v) == 4, pgm_read_dword(v), \
        ({ char error[((sizeof(*v) == 1) || (sizeof(*v) == 2) || \
                        (sizeof(*v) == 4)) ? 1 : -1]; error[0]; }) \

The macro above looks a bit ugly, but it's not too hard to follow. Basically, we are using sizeof(*v) to figure out the size of the data you are wanting, and using it to pick the correct pgm_read_xxx function for the data size. We use __builtin_choose_expr instead of ?: to avoid any unwanted expression evaluations or side effects, as well as to allow different sized return values. The "char error[...]" part is a way to force a compile-time error if the macro is used with something whose size is not 1, 2 or 4 bytes.

The result is that you can write:

char x;
char* PROGMEM FlashPointer = &x;
char* FlashPtr = pgm_read(&FlashPointer);

You can equally write:

static const PROGMEM uint8_t x8 = 8;
static const PROGMEM uint16_t x16 = 16;
static const PROGMEM uint32_t x32 = 32;

void test(void) {
        volatile uint8_t y8 = pgm_read(&x8);
        volatile uint16_t y16 = pgm_read(&x16);
        volatile uint32_t y32 = pgm_read(&x32);

Now your pgm_read's are independent of the type, assuming they have a compatible size.

Personally, my preference would be to change the semantics to remove the "&", so that you would write:

        char* FlashPtr = pgm_read(FlashPointer);

This is just a small change to the macro, but I think it's neater - you are reading an object from flash, rather than reading data from an address in flash. But such a change would require a name change to avoid accidents.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]