search

mate-desktop / mate-power-manager

Power management tool for the MATE desktop
https://mate-desktop.org
GNU General Public License v2.0
59 stars 51 forks source link

Remove compilation warning fix data type format error #394

Closed zhuyaliang closed 9 months ago

raveit65 commented 9 months ago

I found only a few warnings in previous builds but for most changes i do not see any warning. Eg. where do you see data type format error ?

zhuyaliang commented 9 months ago

Compile warnings can appear using meson build

src/gpm-phone.c:394:18: warning: format '%i' expects argument of type 'int', but argument 4 has type 'guint' {aka 'unsigned int'} [-Wformat=]
raveit65 commented 9 months ago

Hmm, normally i use auto-tools and i don't see the warnings with --enable-compile-warnings=maximum. So compiler flags are different in meson :/ Also with meson setup build -Dprefix=/usr -Dwarning_level=everything i don't see this warning.

raveit65 commented 9 months ago

Ok, i see the warnings now.

raveit65 commented 9 months ago

I could confirm most of warnings with

export CFLAGS='-Wsign-compare -Wformat-signedness -Wformat=2 -Wswitch-default -Wcast-function-type -Wdeclaration-after-statement'

./autogen.sh --enable-compile-warnings=maximum

But not all. It's really difficult to review for me when cflags are different with auto-tools and meson. It took me several hours to understand why you want to change parts of the code.

lukefromdc commented 9 months ago

This looks like at some point in the history of this code (presumably not when it was first written...) signed and unsigned integers got mixed up. Yes, this should be fixed.

raveit65 commented 9 months ago

@zhuyaliang Can you please post warnings cflags for next future PR if it so huge again? This will make reviewers life easier

lukefromdc commented 9 months ago

Anyone else want to review this or is it ready to go?

raveit65 commented 9 months ago

I tested it again on my notebook and power management and brightness function are working. Let's merge it.