3party/protobuf-c
0
0
Fork 0
mirror of https://github.com/protobuf-c/protobuf-c.git synced 2024-12-27 22:01:02 +08:00
Code Issues Projects Releases Wiki Activity

protobuf-c: replace DO_ALLOC, FREE, system_alloc, system_free with inline memory allocation functions

Browse Source 
this reworks memory allocation throughout the support library.

the old DO_ALLOC macro had several problems:

  1) only by reading the macro implementation is it possible to tell
  what actually occurs. consider:

        DO_ALLOC(x, ...);

  vs.:

        x = do_alloc(...);

  only in the latter is it clear that x is being assigned to.

  2) it looks like a typical macro/function call, except it alters the
  control flow, usually by return'ing or executing a goto in the
  enclosing function. this type of anti-pattern is explicitly called out
  in the linux kernel coding style.

  3) in one instance, setting the destination pointer to NULL is
  actually a *success* return. in parse_required_member(), when parsing
  a PROTOBUF_C_TYPE_BYTES wire field, it is possible that the field is
  present but of zero length, in which case memory shouldn't be
  allocated and nothing should actually be copied. this is not apparent
  from reading:

        DO_ALLOC(bd->data, allocator, len - pref_len, return 0);
        memcpy(bd->data, data + pref_len, len - pref_len);

  instead, make this behavior explicit:

        if (len - pref_len > 0) {
                bd->data = do_alloc(allocator, len - pref_len);
                if (bd->data == NULL)
                        return 0;
                memcpy(bd->data, data + pref_len, len - pref_len);
        }

  this is much more readable and makes it possible to write a
  replacement for DO_ALLOC which returns NULL on failures.

this changes the protobuf_c_default_allocator to contain only NULL
values; if a replacement function pointer is not present (non-NULL) in
this struct, the default malloc/free implementations are used. this
makes it impossible to call the default allocator functions directly and
represents an API/ABI break, which required a fix to the
PROTOBUF_C_BUFFER_SIMPLE_CLEAR macro.

despite turning one-line allocations in the simple case:

	DO_ALLOC(rv, allocator, desc->sizeof_message, return NULL);

into three-line statements like:

	rv = do_alloc(allocator, desc->sizeof_message);
	if (!rv)
		return (NULL);

this changeset actually *reduces* the total number of lines in the
support library.
This commit is contained in:
Robert Edmonds 2014-01-13 16:43:44 -05:00
parent 8216865a46
commit 09b801a968
2 changed files with 76 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

146
protobuf-c/protobuf-c.c
View File

@ -72,25 +72,6 @@
*/
#define MAX_MEMBERS_FOR_HASH_SIZE 128
/* Try to allocate memory, running some special code if it fails. */
#define DO_ALLOC(dst, allocator, size, fail_code) \
do { \
size_t da__allocation_size = (size); \
if (da__allocation_size == 0) { \
dst = NULL; \
} else if ((dst=((allocator)->alloc((allocator)->allocator_data,\
da__allocation_size))) == NULL) \
{ \
fail_code; \
} \
} while (0)
#define FREE(allocator, ptr) \
do { \
if ((ptr) != NULL) \
((allocator)->free((allocator)->allocator_data, (ptr)));\
} while (0)
#define STRUCT_MEMBER_P(struct_p, struct_offset) \
((void *) ((uint8_t *) (struct_p) + (struct_offset)))
@ -119,25 +100,24 @@ unsigned protobuf_c_minor = PROTOBUF_C_MINOR;
/* --- allocator --- */
static void *
system_alloc(void *allocator_data, size_t size)
static inline void *
do_alloc(ProtobufCAllocator *allocator, size_t size)
{
void *rv;
(void) allocator_data;
if (size == 0)
return NULL;
rv = malloc(size);
return rv;
if (allocator != NULL && allocator->alloc != NULL)
return allocator->alloc(allocator->allocator_data, size);
else
return malloc(size);
}
static void
system_free(void *allocator_data, void *data)
static inline void
do_free(ProtobufCAllocator *allocator, void *data)
{
(void) allocator_data;
if (data)
free(data);
if (data) {
if (allocator != NULL && allocator->free != NULL)
allocator->free(allocator->allocator_data, data);
else
free(data);
}
}
/*
@ -146,8 +126,8 @@ system_free(void *allocator_data, void *data)
* messages.
*/
ProtobufCAllocator protobuf_c_default_allocator = {
.alloc = system_alloc,
.free = system_free,
.alloc = NULL,
.free = NULL,
.allocator_data = NULL,
};
@ -165,11 +145,12 @@ protobuf_c_buffer_simple_append(ProtobufCBuffer *buffer,
uint8_t *new_data;
while (new_alloced < new_len)
new_alloced += new_alloced;
DO_ALLOC(new_data, &protobuf_c_default_allocator,
new_alloced, return);
new_data = do_alloc(NULL, new_alloced);
if (!new_data)
return;
memcpy(new_data, simp->data, simp->len);
if (simp->must_free_data)
FREE(&protobuf_c_default_allocator, simp->data);
do_free(NULL, simp->data);
else
simp->must_free_data = 1;
simp->data = new_data;
@ -1572,10 +1553,10 @@ merge_messages(ProtobufCMessage *earlier_msg,
sizeof_elt_in_repeated_array(fields[i].type);
uint8_t *new_field;
DO_ALLOC(new_field, allocator,
(*n_earlier +
*n_latter) * el_size,
return 0);
new_field = do_alloc(allocator,
(*n_earlier + *n_latter) * el_size);
if (!new_field)
return 0;
memcpy(new_field, *p_latter,
*n_latter * el_size);
@ -1584,8 +1565,8 @@ merge_messages(ProtobufCMessage *earlier_msg,
*p_earlier,
*n_earlier * el_size);
FREE(allocator, *p_latter);
FREE(allocator, *p_earlier);
do_free(allocator, *p_latter);
do_free(allocator, *p_earlier);
*p_latter = new_field;
*n_latter = *n_earlier + *n_latter;
} else {
@ -1914,9 +1895,11 @@ parse_required_member(ScannedMember *scanned_member,
if (maybe_clear && *pstr != NULL) {
const char *def = scanned_member->field->default_value;
if (*pstr != NULL && *pstr != def)
FREE(allocator, *pstr);
do_free(allocator, *pstr);
}
DO_ALLOC(*pstr, allocator, len - pref_len + 1, return 0);
*pstr = do_alloc(allocator, len - pref_len + 1);
if (*pstr == NULL)
return 0;
memcpy(*pstr, data + pref_len, len - pref_len);
(*pstr)[len - pref_len] = 0;
return 1;
@ -1934,10 +1917,14 @@ parse_required_member(ScannedMember *scanned_member,
bd->data != NULL &&
(def_bd == NULL || bd->data != def_bd->data))
{
FREE(allocator, bd->data);
do_free(allocator, bd->data);
}
if (len - pref_len > 0) {
bd->data = do_alloc(allocator, len - pref_len);
if (bd->data == NULL)
return 0;
memcpy(bd->data, data + pref_len, len - pref_len);
}
DO_ALLOC(bd->data, allocator, len - pref_len, return 0);
memcpy(bd->data, data + pref_len, len - pref_len);
bd->len = len - pref_len;
return 1;
}
@ -2177,7 +2164,9 @@ parse_member(ScannedMember *scanned_member,
ufield->tag = scanned_member->tag;
ufield->wire_type = scanned_member->wire_type;
ufield->len = scanned_member->len;
DO_ALLOC(ufield->data, allocator, scanned_member->len, return 0);
ufield->data = do_alloc(allocator, scanned_member->len);
if (ufield->data == NULL)
return 0;
memcpy(ufield->data, scanned_member->data, ufield->len);
return 1;
}
@ -2206,7 +2195,6 @@ parse_member(ScannedMember *scanned_member,
return 0;
}
/*
* TODO: expose/use this function if desc->message_init == NULL (which occurs
* for old code, and may be useful for certain programmatic techniques for
@ -2327,7 +2315,9 @@ protobuf_c_message_unpack(const ProtobufCMessageDescriptor *desc,
#define REQUIRED_FIELD_BITMAP_IS_SET(index) \
(required_fields_bitmap[(index/8)%sizeof(required_fields_bitmap)] & (1<<((index)%8)))
DO_ALLOC(rv, allocator, desc->sizeof_message, return NULL);
rv = do_alloc(allocator, desc->sizeof_message);
if (!rv)
return (NULL);
scanned_member_slabs[0] = first_member_slab;
/*
@ -2445,9 +2435,9 @@ protobuf_c_message_unpack(const ProtobufCMessageDescriptor *desc,
which_slab++;
size = sizeof(ScannedMember)
<< (which_slab + FIRST_SCANNED_MEMBER_SLAB_SIZE_LOG2);
DO_ALLOC(scanned_member_slabs[which_slab],
allocator, size,
goto error_cleanup_during_scan);
scanned_member_slabs[which_slab] = do_alloc(allocator, size);
if (scanned_member_slabs[which_slab] == NULL)
goto error_cleanup_during_scan;
}
scanned_member_slabs[which_slab][in_slab_index++] = tmp;
@ -2498,11 +2488,12 @@ protobuf_c_message_unpack(const ProtobufCMessageDescriptor *desc,
if (field->label == PROTOBUF_C_LABEL_REPEATED) \
STRUCT_MEMBER (size_t, rv, field->quantifier_offset) = 0; \
}
DO_ALLOC(STRUCT_MEMBER
(void *, rv, field->offset),
allocator, siz * n,
CLEAR_REMAINING_N_PTRS();
goto error_cleanup);
void *a = do_alloc(allocator, siz * n);
if (!a) {
CLEAR_REMAINING_N_PTRS();
goto error_cleanup;
}
STRUCT_MEMBER(void *, rv, field->offset) = a;
}
} else if (field->label == PROTOBUF_C_LABEL_REQUIRED) {
if (field->default_value == NULL &&
@ -2519,10 +2510,10 @@ protobuf_c_message_unpack(const ProtobufCMessageDescriptor *desc,
/* allocate space for unknown fields */
if (n_unknown) {
DO_ALLOC(rv->unknown_fields,
allocator,
n_unknown * sizeof(ProtobufCMessageUnknownField),
goto error_cleanup);
rv->unknown_fields = do_alloc(allocator,
n_unknown * sizeof(ProtobufCMessageUnknownField));
if (rv->unknown_fields == NULL)
goto error_cleanup;
}
/* do real parsing */
@ -2544,20 +2535,20 @@ protobuf_c_message_unpack(const ProtobufCMessageDescriptor *desc,
/* cleanup */
for (j = 1; j <= which_slab; j++)
FREE(allocator, scanned_member_slabs[j]);
do_free(allocator, scanned_member_slabs[j]);
return rv;
error_cleanup:
protobuf_c_message_free_unpacked(rv, allocator);
for (j = 1; j <= which_slab; j++)
FREE(allocator, scanned_member_slabs[j]);
do_free(allocator, scanned_member_slabs[j]);
return NULL;
error_cleanup_during_scan:
FREE(allocator, rv);
do_free(allocator, rv);
for (j = 1; j <= which_slab; j++)
FREE(allocator, scanned_member_slabs[j]);
do_free(allocator, scanned_member_slabs[j]);
return NULL;
}
@ -2586,12 +2577,11 @@ protobuf_c_message_free_unpacked(ProtobufCMessage *message,
if (desc->fields[f].type == PROTOBUF_C_TYPE_STRING) {
unsigned i;
for (i = 0; i < n; i++)
FREE(allocator, ((char **) arr)[i]);
do_free(allocator, ((char **) arr)[i]);
} else if (desc->fields[f].type == PROTOBUF_C_TYPE_BYTES) {
unsigned i;
for (i = 0; i < n; i++)
FREE(allocator,
((ProtobufCBinaryData *) arr)[i].data);
do_free(allocator, ((ProtobufCBinaryData *) arr)[i].data);
} else if (desc->fields[f].type == PROTOBUF_C_TYPE_MESSAGE) {
unsigned i;
for (i = 0; i < n; i++)
@ -2601,13 +2591,13 @@ protobuf_c_message_free_unpacked(ProtobufCMessage *message,
);
}
if (arr != NULL)
FREE(allocator, arr);
do_free(allocator, arr);
} else if (desc->fields[f].type == PROTOBUF_C_TYPE_STRING) {
char *str = STRUCT_MEMBER(char *, message,
desc->fields[f].offset);
if (str && str != desc->fields[f].default_value)
FREE(allocator, str);
do_free(allocator, str);
} else if (desc->fields[f].type == PROTOBUF_C_TYPE_BYTES) {
void *data = STRUCT_MEMBER(ProtobufCBinaryData, message,
desc->fields[f].offset).data;
@ -2618,7 +2608,7 @@ protobuf_c_message_free_unpacked(ProtobufCMessage *message,
(default_bd == NULL ||
default_bd->data != data))
{
FREE(allocator, data);
do_free(allocator, data);
}
} else if (desc->fields[f].type == PROTOBUF_C_TYPE_MESSAGE) {
ProtobufCMessage *sm;
@ -2631,11 +2621,11 @@ protobuf_c_message_free_unpacked(ProtobufCMessage *message,
}
for (f = 0; f < message->n_unknown_fields; f++)
FREE(allocator, message->unknown_fields[f].data);
do_free(allocator, message->unknown_fields[f].data);
if (message->unknown_fields != NULL)
FREE(allocator, message->unknown_fields);
do_free(allocator, message->unknown_fields);
FREE(allocator, message);
do_free(allocator, message);
}
void

12
protobuf-c/protobuf-c.h
View File

@ -484,10 +484,14 @@ struct _ProtobufCBufferSimple {
#define PROTOBUF_C_BUFFER_SIMPLE_CLEAR(simp_buf) \
do { \
if ((simp_buf)->must_free_data) \
protobuf_c_default_allocator.free( \
&protobuf_c_default_allocator.allocator_data, \
(simp_buf)->data); \
if ((simp_buf)->must_free_data) { \
if (protobuf_c_default_allocator.free == NULL) \
free((simp_buf)->data); \
else \
protobuf_c_default_allocator.free( \
&protobuf_c_default_allocator, \
simp_buf); \
} \
} while (0)
#ifdef PROTOBUF_C_PLEASE_INCLUDE_CTYPE