On Wed, Sep 15, 2010 at 8:07 AM, Allan McRae <[email protected]> wrote: > On 13/09/10 00:01, Sebastian Nowicki wrote: >> >> -Wbad-function-cast: >> * casting long int to pmpkgreason_t unecessarily >> * casting pmpkgreason_t to long unecessarily >> * casting off_t (signed integral) to float before division >> unecessarily >> >> -Wshadow: >> * shadowing "handle" variables renamed to "new_handle" and >> "local_handle" shadowing "filestr" renamed to "pkgfilestr" >> * shadowing "remove" renamed to "remove_pkgs" >> * shadowing "sync" renamed to "syncpkg" >> * Removed redundant declarations >> * shadowing "prefix" renamed to "entry_prefix" >> * shadowing "pipe" renamed to "pipe_handle" >> >> -Wconversion: >> * explicitely cast nread to size_t from ssize_t in dload.c >> (guaranteed not to be negative at this point) >> * use size_t, as opposed to int, for string length and byte sizes >> * use signed int in alpm_list_count as advertised by API (should >> probably be changed to usngiend int or size_t, but requires an >> API >> change) >> >> Signed-off-by: Sebastian Nowicki<[email protected]> > > I tried having a look at this but kept getting lost in what part of the > patch fixed what warnings. e.g. I can not find the casting off_t to float > change for -Wbad-function-cast. > > I think it would be much better to split these up a bit more. Splitting by > error type would be fine for me.
+1; if you could do one patch for each warning that would be awesome. I know it is a bit more work but it makes it a lot easier to see what you've done (as well as prevent it from sneaking in again in the future). > >> --- >> Some of these warnings are quite pedantic and wouldn't really cause >> issues, so they might not be desired. >> >> The change in alpm_list_count is backwards in my opinion - the API >> should be changed to use unsigned int (or size_t). I didn't want to >> change the API though. > > I can not see why this ever returned a signed int. This should really be size_t, just like strlen(), etc. If we want to change it in a major version release, we haven't shied away from it in the past, so I'd be for it. In most cases it wouldn't break code or built applications anyway. -Dan
