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

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Wed Jan 11 15:06:06 UTC 2017


Patch Set 3:

I can only reiterate my previous comment:

In short: set to INVALID only where it is supposed to be INVALID, and/or, input-check on that called function that potentially receives the INVALID items. It is currently unclear whether the code will happily use INVALID items as if they were valid. That should be easy to see.

In long:

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

Let's stop discussing and see another patch set... In case your intention is to abandon this, please clearly say so; but I assume it's not much effort and would appreciate if you could follow up.

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