From 840e213b9fa244bce4c67789971c02ce16e66295 Mon Sep 17 00:00:00 2001 From: Petteri Aimonen Date: Wed, 11 Sep 2013 09:53:51 +0300 Subject: [PATCH 1/2] Get rid of the ternary operator in the pb_field_t initialization. Some compilers where unable to detect that the ternary operator can be evaluated at the compile time. This commit does the evaluation on the Python side, which should fix the problem. The new .pb.c files are generated using PB_FIELD2() macro. The old PB_FIELD() macro remains, so that previously generated files keep working. --- generator/nanopb_generator.py | 3 +- pb.h | 56 +++++++++++++++++++++++------------ 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py index d70938a..e463b6c 100755 --- a/generator/nanopb_generator.py +++ b/generator/nanopb_generator.py @@ -246,13 +246,14 @@ class Field: '''Return the pb_field_t initializer to use in the constant array. prev_field_name is the name of the previous field or None. ''' - result = ' PB_FIELD(%3d, ' % self.tag + result = ' PB_FIELD2(%3d, ' % self.tag result += '%-8s, ' % self.pbtype result += '%s, ' % self.rules result += '%s, ' % self.allocation result += '%s, ' % self.struct_name result += '%s, ' % self.name result += '%s, ' % (prev_field_name or self.name) + result += '%s, ' % ("first" if not prev_field_name else "other") if self.pbtype == 'MESSAGE': result += '&%s_fields)' % self.submsgname diff --git a/pb.h b/pb.h index e3e68ce..fe91ccd 100644 --- a/pb.h +++ b/pb.h @@ -320,58 +320,66 @@ struct _pb_extension_t { }; /* These macros are used to declare pb_field_t's in the constant array. */ +/* Size of a structure member, in bytes. */ #define pb_membersize(st, m) (sizeof ((st*)0)->m) +/* Number of entries in an array. */ #define pb_arraysize(st, m) (pb_membersize(st, m) / pb_membersize(st, m[0])) +/* Delta from start of one member to the start of another member. */ #define pb_delta(st, m1, m2) ((int)offsetof(st, m1) - (int)offsetof(st, m2)) -#define pb_delta_end(st, m1, m2) (int)(offsetof(st, m1) == offsetof(st, m2) \ - ? offsetof(st, m1) \ - : offsetof(st, m1) - offsetof(st, m2) - pb_membersize(st, m2)) +/* Delta from start of structure to member. */ +#define pb_fielddelta_first(st, m1, m2) (offsetof(st, m1)) +/* Delta from end of one field to start of another field. */ +#define pb_fielddelta_other(st, m1, m2) (offsetof(st, m1) - offsetof(st, m2) - pb_membersize(st, m2)) +/* Choose between pb_fielddelta_first and pb_fielddelta_other (backwards compatibility) */ +#define pb_fielddelta_choose(st, m1, m2) (int)(offsetof(st, m1) == offsetof(st, m2) \ + ? pb_fielddelta_first(st, m1, m2) \ + : pb_fielddelta_other(st, m1, m2)) #define PB_LAST_FIELD {0,(pb_type_t) 0,0,0,0,0,0} /* Required fields are the simplest. They just have delta (padding) from * previous field end, and the size of the field. Pointer is used for * submessages and default values. */ -#define PB_REQUIRED_STATIC(tag, st, m, pm, ltype, ptr) \ +#define PB_REQUIRED_STATIC(tag, st, m, fd, ltype, ptr) \ {tag, PB_ATYPE_STATIC | PB_HTYPE_REQUIRED | ltype, \ - pb_delta_end(st, m, pm), 0, pb_membersize(st, m), 0, ptr} + fd, 0, pb_membersize(st, m), 0, ptr} /* Optional fields add the delta to the has_ variable. */ -#define PB_OPTIONAL_STATIC(tag, st, m, pm, ltype, ptr) \ +#define PB_OPTIONAL_STATIC(tag, st, m, fd, ltype, ptr) \ {tag, PB_ATYPE_STATIC | PB_HTYPE_OPTIONAL | ltype, \ - pb_delta_end(st, m, pm), \ + fd, \ pb_delta(st, has_ ## m, m), \ pb_membersize(st, m), 0, ptr} /* Repeated fields have a _count field and also the maximum number of entries. */ -#define PB_REPEATED_STATIC(tag, st, m, pm, ltype, ptr) \ +#define PB_REPEATED_STATIC(tag, st, m, fd, ltype, ptr) \ {tag, PB_ATYPE_STATIC | PB_HTYPE_REPEATED | ltype, \ - pb_delta_end(st, m, pm), \ + fd, \ pb_delta(st, m ## _count, m), \ pb_membersize(st, m[0]), \ pb_arraysize(st, m), ptr} /* Callbacks are much like required fields except with special datatype. */ -#define PB_REQUIRED_CALLBACK(tag, st, m, pm, ltype, ptr) \ +#define PB_REQUIRED_CALLBACK(tag, st, m, fd, ltype, ptr) \ {tag, PB_ATYPE_CALLBACK | PB_HTYPE_REQUIRED | ltype, \ - pb_delta_end(st, m, pm), 0, pb_membersize(st, m), 0, ptr} + fd, 0, pb_membersize(st, m), 0, ptr} -#define PB_OPTIONAL_CALLBACK(tag, st, m, pm, ltype, ptr) \ +#define PB_OPTIONAL_CALLBACK(tag, st, m, fd, ltype, ptr) \ {tag, PB_ATYPE_CALLBACK | PB_HTYPE_OPTIONAL | ltype, \ - pb_delta_end(st, m, pm), 0, pb_membersize(st, m), 0, ptr} + fd, 0, pb_membersize(st, m), 0, ptr} -#define PB_REPEATED_CALLBACK(tag, st, m, pm, ltype, ptr) \ +#define PB_REPEATED_CALLBACK(tag, st, m, fd, ltype, ptr) \ {tag, PB_ATYPE_CALLBACK | PB_HTYPE_REPEATED | ltype, \ - pb_delta_end(st, m, pm), 0, pb_membersize(st, m), 0, ptr} + fd, 0, pb_membersize(st, m), 0, ptr} /* Optional extensions don't have the has_ field, as that would be redundant. */ -#define PB_OPTEXT_STATIC(tag, st, m, pm, ltype, ptr) \ +#define PB_OPTEXT_STATIC(tag, st, m, fd, ltype, ptr) \ {tag, PB_ATYPE_STATIC | PB_HTYPE_OPTIONAL | ltype, \ 0, \ 0, \ pb_membersize(st, m), 0, ptr} -#define PB_OPTEXT_CALLBACK(tag, st, m, pm, ltype, ptr) \ +#define PB_OPTEXT_CALLBACK(tag, st, m, fd, ltype, ptr) \ {tag, PB_ATYPE_CALLBACK | PB_HTYPE_OPTIONAL | ltype, \ 0, 0, pb_membersize(st, m), 0, ptr} @@ -410,8 +418,18 @@ struct _pb_extension_t { */ #define PB_FIELD(tag, type, rules, allocation, message, field, prevfield, ptr) \ - PB_ ## rules ## _ ## allocation(tag, message, field, prevfield, \ - PB_LTYPE_MAP_ ## type, ptr) + PB_ ## rules ## _ ## allocation(tag, message, field, \ + pb_fielddelta_choose(message, field, prevfield), \ + PB_LTYPE_MAP_ ## type, ptr) + +/* This is a new version of the macro used by nanopb generator from + * version 0.2.3 onwards. It avoids the use of a ternary expression in + * the initialization, which confused some compilers. + */ +#define PB_FIELD2(tag, type, rules, allocation, message, field, prevfield, pos, ptr) \ + PB_ ## rules ## _ ## allocation(tag, message, field, \ + pb_fielddelta_ ## pos(message, field, prevfield), \ + PB_LTYPE_MAP_ ## type, ptr) /* These macros are used for giving out error messages. From 9ada7e752516260054525fca8e1f67efa321f682 Mon Sep 17 00:00:00 2001 From: Petteri Aimonen Date: Fri, 13 Sep 2013 11:30:58 +0300 Subject: [PATCH 2/2] Fine-tune the naming of new macros before merging into master. Requires re-generation of files generated with dev_get_rid_of_ternary_operator. --- generator/nanopb_generator.py | 2 +- pb.h | 28 +++++++++++++++++----------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py index e463b6c..2e30b67 100755 --- a/generator/nanopb_generator.py +++ b/generator/nanopb_generator.py @@ -250,10 +250,10 @@ class Field: result += '%-8s, ' % self.pbtype result += '%s, ' % self.rules result += '%s, ' % self.allocation + result += '%s, ' % ("FIRST" if not prev_field_name else "OTHER") result += '%s, ' % self.struct_name result += '%s, ' % self.name result += '%s, ' % (prev_field_name or self.name) - result += '%s, ' % ("first" if not prev_field_name else "other") if self.pbtype == 'MESSAGE': result += '&%s_fields)' % self.submsgname diff --git a/pb.h b/pb.h index fe91ccd..98b9bbd 100644 --- a/pb.h +++ b/pb.h @@ -326,16 +326,19 @@ struct _pb_extension_t { #define pb_arraysize(st, m) (pb_membersize(st, m) / pb_membersize(st, m[0])) /* Delta from start of one member to the start of another member. */ #define pb_delta(st, m1, m2) ((int)offsetof(st, m1) - (int)offsetof(st, m2)) -/* Delta from start of structure to member. */ -#define pb_fielddelta_first(st, m1, m2) (offsetof(st, m1)) -/* Delta from end of one field to start of another field. */ -#define pb_fielddelta_other(st, m1, m2) (offsetof(st, m1) - offsetof(st, m2) - pb_membersize(st, m2)) -/* Choose between pb_fielddelta_first and pb_fielddelta_other (backwards compatibility) */ -#define pb_fielddelta_choose(st, m1, m2) (int)(offsetof(st, m1) == offsetof(st, m2) \ - ? pb_fielddelta_first(st, m1, m2) \ - : pb_fielddelta_other(st, m1, m2)) +/* Marks the end of the field list */ #define PB_LAST_FIELD {0,(pb_type_t) 0,0,0,0,0,0} +/* Macros for filling in the data_offset field */ +/* data_offset for first field in a message */ +#define PB_DATAOFFSET_FIRST(st, m1, m2) (offsetof(st, m1)) +/* data_offset for subsequent fields */ +#define PB_DATAOFFSET_OTHER(st, m1, m2) (offsetof(st, m1) - offsetof(st, m2) - pb_membersize(st, m2)) +/* Choose first/other based on m1 == m2 (deprecated, remains for backwards compatibility) */ +#define PB_DATAOFFSET_CHOOSE(st, m1, m2) (int)(offsetof(st, m1) == offsetof(st, m2) \ + ? PB_DATAOFFSET_FIRST(st, m1, m2) \ + : PB_DATAOFFSET_OTHER(st, m1, m2)) + /* Required fields are the simplest. They just have delta (padding) from * previous field end, and the size of the field. Pointer is used for * submessages and default values. @@ -419,16 +422,19 @@ struct _pb_extension_t { #define PB_FIELD(tag, type, rules, allocation, message, field, prevfield, ptr) \ PB_ ## rules ## _ ## allocation(tag, message, field, \ - pb_fielddelta_choose(message, field, prevfield), \ + PB_DATAOFFSET_CHOOSE(message, field, prevfield), \ PB_LTYPE_MAP_ ## type, ptr) /* This is a new version of the macro used by nanopb generator from * version 0.2.3 onwards. It avoids the use of a ternary expression in * the initialization, which confused some compilers. + * + * - Placement: FIRST or OTHER, depending on if this is the first field in structure. + * */ -#define PB_FIELD2(tag, type, rules, allocation, message, field, prevfield, pos, ptr) \ +#define PB_FIELD2(tag, type, rules, allocation, placement, message, field, prevfield, ptr) \ PB_ ## rules ## _ ## allocation(tag, message, field, \ - pb_fielddelta_ ## pos(message, field, prevfield), \ + PB_DATAOFFSET_ ## placement(message, field, prevfield), \ PB_LTYPE_MAP_ ## type, ptr)