Change in libosmocore[master]: add contrib/struct_endianess.py

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.org
Fri Nov 16 10:41:23 UTC 2018


Pau 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>


More information about the gerrit-log mailing list