osmo-pcu[master]: Sanitizer build fix for invalid value of variable of type eg...

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/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Fri Dec 16 11:32:14 UTC 2016


Patch Set 3:

> I would suggest not to jump into verdict before having proper
 > analysis of code.

You're right that I had in fact not understood the code completely.
Now that I'm reading it in more detail, I only find more problems.
I will limit the comments here on the ones related to this patch:

My point remains that in gprs_rlc_mcs_cps() the punct/punct2 are used in
calculations directly without checking against INVALID items.
By initializing both punct values to "INVALID" in create_dl_acked_block(),
it is suggested that INVALID values may be passed on to gprs_rlc_mcs_cps()
-- if that weren't the case, it would not be necessary to initialize to
INVALID. So, gprs_rlc_mcs_cps() must check that it isn't using INVALID
enum values in its bitfield calculations. Would it make more sense
to set punct2 to INVALID explicitly in specific cases?

Even though incidentally, the cases where one (or both??) args are INVALID
might match the internal expectations of the switch() statement, this is
bad coding style. Each function's API should make sense on its own.

Thanks!

-- 
To view, visit https://gerrit.osmocom.org/1411
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ice54edc7e4a936eb2f2dd8a243673a30dceef542
Gerrit-PatchSet: 3
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: arvind.sirsikar <arvind.sirsikar at radisys.com>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: arvind.sirsikar <arvind.sirsikar at radisys.com>
Gerrit-HasComments: No



More information about the gerrit-log mailing list