[Bug 670302] Review Request: libbacklight - Linux backlight abstraction library

bugzilla at redhat.com bugzilla at redhat.com
Tue Jan 18 04:27:31 UTC 2011

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


Ralf Corsepius <rc040203 at freenet.de> changed:

           What    |Removed                     |Added
                 CC|                            |rc040203 at freenet.de

--- Comment #1 from Ralf Corsepius <rc040203 at freenet.de> 2011-01-17 23:27:30 EST ---

a) To mjg59 in his role as upstream:

- libbacklight.h lacks extern "C" guards.
=> API is not C++ safe.

- libbacklight.c suffers from implicit decls:

libbacklight.c: In function 'backlight_get':
libbacklight.c:74:2: warning: implicit declaration of function 'read'
libbacklight.c:80:2: warning: implicit declaration of function 'strtol'
libbacklight.c:83:2: warning: implicit declaration of function 'close'
libbacklight.c: In function 'backlight_set_brightness':
libbacklight.c:125:2: warning: implicit declaration of function 'write'
=> Code is like to suffer from portability (type-size) issues.
You'll likely want to "#include <unistd.h>"

- Consider to a licensing headers to all source files and a LICENCE/COPYING

- Consider to provide at least a minimum amount of documentation.
ATM, there is no trace of documentation, not even comments in libbacklight.h
describing what these functions are supposed to do.

b) to mjg59 in his role as packager:

- configure.ac applies AM_SILENT_RULES.
=> AM_SILENT_RULES causes build.log to skip essential information (e.g.
compiler calls). I'd recommend upstream to remove AM_SILENT_RULES from
configure.ac. Alternatively, add --disable-silent-rules to %configure

- package applies pkg-config
=> Missing BR: pkg-config
(The fact pkg-config is implicitly being pulled in, is a random coincidence)

- Unnecessary "BR: libtool".
Please remove it, It's not needed.

Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

More information about the package-review mailing list