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/.
Harald Welte gerrit-no-reply at lists.osmocom.org
Patch Set 5: Code-Review-1
(14 comments)
https://gerrit.osmocom.org/#/c/638/5/openbsc/include/openbsc/gprs_llc_xid.h
File openbsc/include/openbsc/gprs_llc_xid.h:
Line 40: int gprs_llc_compile_xid(const struct llist_head *xid_fields, uint8_t *dst,
the copy_xid and parse_xid functions have the dst as first field, and src as second (which is general argument ordering in osmocom). compile_xid has them reversed. Please make sure this is consistent.
https://gerrit.osmocom.org/#/c/638/5/openbsc/src/gprs/gprs_llc_xid.c
File openbsc/src/gprs/gprs_llc_xid.c:
Line 83: talloc_zero_size(NULL, xid_field->data_len);
talloc context comment not yet adressed
Line 84: memcpy(xid_field->data, src, xid_field->data_len);
talloc_memdup comment not yet adressed
Line 179: gprs_llc_free_xid(xid_fields);
extra whitespace at end of line
Line 216: if (xid_fields) {
save one indent level by writing "if (!xid_fields) \n return"
Line 218: if ((xid_field->data) && (xid_field->data_len))
if xid_field->data was allocated from the xid_field as talloc context, this explicit free can go.
Line 255: void gprs_llc_copy_xid(struct llist_head *xid_fields_copy,
again whitespace at EOL. Please keep watching that before pushing.
https://gerrit.osmocom.org/#/c/638/5/openbsc/src/gprs/gprs_sndcp_comp_entity.c
File openbsc/src/gprs/gprs_sndcp_comp_entity.c:
PS5, Line 39: e
no space before parentheis in function definition or call
Line 42: comp_entity = talloc_zero (NULL, struct gprs_sndcp_comp_entity);
extra space, once more
Line 53: memcpy (comp_entity->nsapi,
and again ...
Line 91: /* Determine of which class our compression entity will be
extra white space at end of line. please make sure such easy to spot (and easy to automatically spot) issues are not taking up your and my review time.
Line 109: else
we use curly braces here as there are multiple lines (even if only in a single statement)
Line 121: if (comp_entities) {
"if (!comp_entities)\n return;"
Line 151: if (comp_entities) {
if (!comp_entities)\n return;
--
To view, visit https://gerrit.osmocom.org/638
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia06e4cb08bf9b48c2a4682606d1b1a91d19a9d37
Gerrit-PatchSet: 5
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes