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
Thu Dec 15 11:14:23 UTC 2016


Patch Set 3: Code-Review-1

(2 comments)

https://gerrit.osmocom.org/#/c/1411/3//COMMIT_MSG
Commit Message:

Line 7: Sanitizer build fix for invalid value of variable of type egprs_puncturing_values
Now that I've looked at the patch in more detail, I notice another problem with this commit log.

Let's look at what this is: the sanitizer build shows a failure. The sanitizer build is not a goal in itself, it shows bugs that we fix. This patch suggests that you've found an actual bug in the code, hence the commit log should focus on the bug. What was wrong before, how did you fix it?

Mentioning the error fixed in the sanitizer build is also good to mention, but should not be the center stage of this commit log.


https://gerrit.osmocom.org/#/c/1411/3/src/tbf_dl.cpp
File src/tbf_dl.cpp:

Line 631: 					{EGPRS_PS_INVALID};
Are you aware that you're initializing only the first element of the array to _INVALID? The rest are set to zero instead, incidentally EGPRS_PS_1.

Observe the output of

  #include <stdio.h>
  int main(void){
    int foo[10] = {23};
    int i;
    for (i = 0; i < 10; i++)
      printf("[%d] = %d\n", i, foo[i]);
    return 0;
  }

Furthermore it looks to me like this is a cover-up for a real failure. Somewhere below, it seems to me, an element of this array is failed to be initialized, although the general logic looks like we expect each element to be initialized individually. By initializing to "invalid"/zero, we don't run into the sanitizer failure because the value is now a valid enum val, but we've not fixed the missing initialization issue at all. Am I guessing right?


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



More information about the gerrit-log mailing list