Skip to content

Commit

Permalink
Fix type-punning issues exposed with GCC 4.5.1
Browse files Browse the repository at this point in the history
The errors below are due to pointer magic that isn't allowed if following C
strict-aliasing rules:

    memcached.c: In function ‘complete_incr_bin’:
    memcached.c:1023:16: error: dereferencing type-punned pointer will break
    strict-aliasing rules
    memcached.c:1044:13: error: dereferencing type-punned pointer will break
    strict-aliasing rules
    memcached.c:1061:17: error: dereferencing type-punned pointer will break
    strict-aliasing rules

Fix this by introducing a union type that allows access to the uint64_t
member as necessary, but doesn't add any additional length to the structure.
The size remains the same before and after; the only difference is explict
casts are now refactored into union member accesses and all compilers should
be happy.

Signed-off-by: Dan McGee <[email protected]>
  • Loading branch information
toofishes authored and dustin committed Nov 3, 2010
1 parent 92d6e12 commit df15887
Showing 1 changed file with 17 additions and 8 deletions.
25 changes: 17 additions & 8 deletions memcached.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,22 @@
#define TAIL_REPAIR_TIME (3 * 3600)

/* warning: don't use these macros with a function, as it evals its arg twice */
#define ITEM_get_cas(i) ((uint64_t)(((i)->it_flags & ITEM_CAS) ? \
*(uint64_t*)&((i)->end[0]) : 0x0))
#define ITEM_set_cas(i,v) { if ((i)->it_flags & ITEM_CAS) { \
*(uint64_t*)&((i)->end[0]) = v; } }
#define ITEM_get_cas(i) (((i)->it_flags & ITEM_CAS) ? \
(i)->data->cas : (uint64_t)0)

#define ITEM_key(item) (((char*)&((item)->end[0])) \
#define ITEM_set_cas(i,v) { \
if ((i)->it_flags & ITEM_CAS) { \
(i)->data->cas = v; \
} \
}

#define ITEM_key(item) (((char*)&((item)->data)) \
+ (((item)->it_flags & ITEM_CAS) ? sizeof(uint64_t) : 0))

#define ITEM_suffix(item) ((char*) &((item)->end[0]) + (item)->nkey + 1 \
#define ITEM_suffix(item) ((char*) &((item)->data) + (item)->nkey + 1 \
+ (((item)->it_flags & ITEM_CAS) ? sizeof(uint64_t) : 0))

#define ITEM_data(item) ((char*) &((item)->end[0]) + (item)->nkey + 1 \
#define ITEM_data(item) ((char*) &((item)->data) + (item)->nkey + 1 \
+ (item)->nsuffix \
+ (((item)->it_flags & ITEM_CAS) ? sizeof(uint64_t) : 0))

Expand Down Expand Up @@ -302,7 +306,12 @@ typedef struct _stritem {
uint8_t it_flags; /* ITEM_* above */
uint8_t slabs_clsid;/* which slab class we're in */
uint8_t nkey; /* key length, w/terminating null and padding */
void * end[];
/* this odd type prevents type-punning issues when we do
* the little shuffle to save space when not using CAS. */
union {
uint64_t cas;
char end;
} data[];
/* if it_flags & ITEM_CAS we have 8 bytes CAS */
/* then null-terminated key */
/* then " flags length\r\n" (no terminating null) */
Expand Down

0 comments on commit df15887

Please sign in to comment.