Hi.
I think it's better to fix those warnings instead of rolling back commit. It was added because without it there were plenty of warnings in other places so overall we won't decrease number of warnings - we'll just shift them from one place to another. Besides if we follow that logic we have to rollback f45334be29016a36594aacc07c90e262e4994525 for example.
Regarding the potential fix - it pretty-much boild down to interaction between strtok_r() and talloc(). The obvious way would be to talloc_strdup() string before passing it to 1st call to strtok_r(). The potential problem is that strtok_r() changes it's argument. So the questions are: - do we expect that strtok_r() changes to 1st argument? If so - what kind of changes are expected? - what does strtok_r() do with memory? When it is safe to remove duplicated string passed to it?
On 10/18/2016 02:15 AM, Neels Hofmeyr wrote:
Presumably after this libosmocore commit, I get scores of new warnings (see below) when compiling openbsc:
commit ed9d6da5df98538adc70aa03cb569eb9505d04b6 Author: Max msuraev@sysmocom.de AuthorDate: Tue Oct 11 15:20:28 2016 +0200
Constify ctrl_cmd struct fields where appropriateSome of these can be fixed by constifying the functions as well, but talloc_free() of a const char * looks really wrong?
Please follow up on this or revert the commit... constifying is good, but having twice the amount of warnings means twice the reading effort during development cycles.
I would most prefer a revert now and come back as soon as no new warnings are introduced; ymmv...
In file included from /usr/local/include/osmocom/core/talloc.h:4:0, from ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c:22: ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c: In function ‘bsc_nat_handle_ctrlif_msg’: ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c:131:16: warning: passing argument 1 of ‘_talloc_free’ discards ‘const’ qualifier from pointer target type talloc_free(cmd->variable); ^ /usr/include/talloc.h:227:5: note: expected ‘void *’ but argument is of type ‘const char *’ int _talloc_free(void *ptr, const char *location); ^ ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c:143:34: warning: passing argument 2 of ‘bsc_get_pending’ discards ‘const’ qualifier from pointer target type pending = bsc_get_pending(bsc, cmd->id); ^ ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c:86:29: note: expected ‘char *’ but argument is of type ‘const char *’ static struct bsc_cmd_list *bsc_get_pending(struct bsc_connection *bsc, char *id_str) ^ ../../../src/osmo-bsc_nat/bsc_ussd.c: In function ‘bsc_ussd_check’: ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c: In function ‘forward_to_bsc’: ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c:228:31: warning: passing argument 1 of ‘extract_bsc_nr_variable’ discards ‘const’ qualifier from pointer target type if (!extract_bsc_nr_variable(cmd->variable, &nr, &bsc_variable)) { ^ ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c:198:12: note: expected ‘char *’ but argument is of type ‘const char *’ static int extract_bsc_nr_variable(char *variable, unsigned int *nr, char **bsc_variable) ^ In file included from /usr/local/include/osmocom/core/talloc.h:4:0, from ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c:22: ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c:259:16: warning: passing argument 1 of ‘_talloc_free’ discards ‘const’ qualifier from pointer target type talloc_free(bsc_cmd->id); ^ /usr/include/talloc.h:227:5: note: expected ‘void *’ but argument is of type ‘const char *’ int _talloc_free(void *ptr, const char *location); ^ ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c:266:16: warning: passing argument 1 of ‘_talloc_free’ discards ‘const’ qualifier from pointer target type talloc_free(bsc_cmd->variable); ^ /usr/include/talloc.h:227:5: note: expected ‘void *’ but argument is of type ‘const char *’ int _talloc_free(void *ptr, const char *location); ^ ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c: In function ‘extract_bsc_cfg_variable’: ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c:323:31: warning: passing argument 1 of ‘extract_bsc_nr_variable’ discards ‘const’ qualifier from pointer target type if (!extract_bsc_nr_variable(cmd->variable, &nr, bsc_variable)) { ^ ../../../src/osmo-bsc_nat/bsc_nat_ctrl.c:198:12: note: expected ‘char *’ but argument is of type ‘const char *’ static int extract_bsc_nr_variable(char *variable, unsigned int *nr, char **bsc_variable) ^