[pacman-dev] [PATCH 1/2] Fix warnings not usually emitted (CFLAGS not in configure.ac)

Dan McGee dpmcgee at gmail.com
Wed Sep 15 09:30:06 EDT 2010

On Wed, Sep 15, 2010 at 8:07 AM, Allan McRae <allan at archlinux.org> 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<sebnow at gmail.com>
> 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

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


More information about the pacman-dev mailing list