On 16/10/16 01:15, Lars Schneider wrote: >> On 15 Oct 2016, at 14:01, Ramsay Jones <ram...@ramsayjones.plus.com> wrote: >> On 15/10/16 16:05, Lars Schneider wrote: >>>> On 11 Oct 2016, at 16:46, Ramsay Jones <ram...@ramsayjones.plus.com> wrote: >> [snip] >>>> -void stop_multi_file_filter(struct child_process *process) >>>> +static void stop_multi_file_filter(struct child_process *process) >>> >>> Done! Do you have some kind of script to detect these things >>> automatically or do you read the code that carefully? >> >> Heh, I'm _far_ too lazy to read the code that carefully. :-D >> >> A combination of 'make sparse' and a perl script (originally >> posted to the list by Junio) find all of these for me. > > Interesting. Do you have a link to this script?
I'm attaching _a_ version of the script (you would think that I would have it under version control; but no, I copy versions around the several machines/git.git repos I have ... :-P ). > Does it generate false positives? Hmm, well, you have to remember that 'make clean' sometimes doesn't make clean. Ever since the Makefile was changed to only remove $(OBJECTS), rather than *.o xdiff/*.o etc., you have to remember to 'make clean' _before_ you switch branches. Otherwise, you risk leaving some objects laying around. Since the script runs 'nm' on all objects it finds, any stale ones can cause problems. (Of course, I almost always forget, so I frequently have to manually check for and remove stale objects!) Also, you have to interpret the results of the script. Just because it shows you an unused external, that doesn't make it an error. The way I use it, BTW, is to run it on a freshly built branch (saving to a file) and then compare the files from master->next->pu. I don't bother looking at the output on master (the 'stop list' in the script is _way_ out of date), just the diffs master->next and next->pu. The current next->pu diff looks like: $ diff nsc psc 0a1,2 > attr.o - attr_name_valid > attr.o - invalid_attr_name_message 17a20 > convert.o - stop_multi_file_filter 34d36 < quote.o - sq_quotef 43a46 > sequencer.o - append_new_todo 50a54 > trailer.o - conf_head 55a60,61 > wt-status.o - has_uncommitted_changes > wt-status.o - has_unstaged_changes $ I have already sent mails about 'stop_multi_file_filter', 'append_new_todo' and 'conf_head'. The symbols in attr.o are part of the revamp of the attribute system that Junio and Stefan are working on, which is very much in flux. If they are still there when it looks to have firmed up, then I will take a look. The 'sq_quotef' symbol is an example of a useful function that was written without having an immediate user (actually, if memory serves correctly, it was originally used in the series which introduced it, but later modifications removed the caller). It now has a user. The wt-status.o symbols have been introduced by Johannes in his 'rebase -i' series (of series) and I am assuming that these symbols will find additional external callers in up-coming series. > Maybe I can add `make sparse` to the TravisCI build? I don't know how feasible that would be. Apparently, the version of sparse in most distribution repositories is so old that it spews so many spurious errors as to make it unusable. See, for example: https://public-inbox.org/git/56ca5753.9030...@ramsayjones.plus.com/ So, you would need to build from source to get a usable version (I am currently running a version built with some local patches). If you were to build from the sparse repo's master branch, then it would work great on Linux, at least. I have no idea if it would work on mac OS X. However, even then, a single sparse warning is issued on Linux (for the master branch, several others on pu): SP pack-revindex.c pack-revindex.c:65:23: warning: memset with byte count of 262144 This is a problem with sparse (the byte count limit used is hardcoded into sparse), but I haven't sent a patch upstream to fix it yet. (I've been looking at that warning for _many_ years, so don't hold your breath!) ATB, Ramsay Jones
Description: Perl program