This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Pau Espin Pedrol gerrit-no-reply at lists.osmocom.orgPau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/11786 ) Change subject: add contrib/struct_endianess.py ...................................................................... Patch Set 2: (13 comments) https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py File contrib/struct_endianess.py: https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@37 PS2, Line 37: def struct_body_to_big_endian(body_str): Document what this function does exactly. https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@80 PS2, Line 80: # now the interesting part, go over the defs, and reverse the sub-byte ints You can probably split into a different function starting from here. https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@118 PS2, Line 118: raise Exception('sub-byte int breaks clean byte bounds: %s -- %d + %d = %d bits' what about eg. bitfield for uint16_t? are they expected to be reversed too in big endian? https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@168 PS2, Line 168: sections.append(buf) shouldn't appending to sections be done at the end of the struct only? (line 173). https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@181 PS2, Line 181: # examine each struct, i.e. every second item in 'sections' Ah I see now, the one you append is actually the buffer until before the struct code starts. Worth explicitly commenting that during first "for" loop I think, specially comment "why" you want to have both at that point. https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@189 PS2, Line 189: if not 'packed' in struct[-1]: probably better/easier to look for __atribute((packed)) or whatever is called, and check all lines (to avoid possible corner cases with whitespace or whatever). https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@199 PS2, Line 199: # divide in struct body sections of Probably worth moving this into another function to make code easier to read and maintain. https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@200 PS2, Line 200: # ['arbitrary string', ['body;\n', 'lines;\n'], 'arbitrary string'] what does body and lines means? brief explanation here welcome. https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@208 PS2, Line 208: def end_def(): Please document this function and global variables below. https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@224 PS2, Line 224: or re_substruct_end.fullmatch(line)): I think this can go in same line https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@248 PS2, Line 248: j += 1 Not sure if this j += 1 in here applied for all cases is correct. Have a look. https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@276 PS2, Line 276: end_def() afaiu this end_def() should be one lne above and indented one more level (inside while). https://gerrit.osmocom.org/#/c/11786/2/contrib/struct_endianess.py@308 PS2, Line 308: raise Exception('do not know where to include osmocom/core/endian.h in %r' % f) Just add it around first empty line (to avoid adding if before guard IFDEF or pragma once). -- To view, visit https://gerrit.osmocom.org/11786 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e75b17d8071c7b3a2a171ba776fb76854b28a53 Gerrit-Change-Number: 11786 Gerrit-PatchSet: 2 Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-CC: Pau Espin Pedrol <pespin at sysmocom.de> Gerrit-Comment-Date: Fri, 16 Nov 2018 10:41:23 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181116/40007e23/attachment.htm>