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

bugzilla at redhat.com bugzilla at redhat.com
Wed Jan 19 03:03:48 UTC 2011


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


https://bugzilla.redhat.com/show_bug.cgi?id=670302

--- Comment #3 from Ralf Corsepius <rc040203 at freenet.de> 2011-01-18 22:03:48 EST ---
(In reply to comment #2)
> unistd.h added (not sure why I'm not getting those warnings)
I was building the package in f14 and f15-x86_64 mock-chroots on f14-x86_64
=> package received RPM_OPTS_FLAGS.

Likely you are building in an ordinary user's environment (The files inside of
your srpm being owned by user mjg59 indicates this) without explict explicitly
setting CFLAGS etc.

[NB: This is one of the situations AM_SILENT_RULES exposes its harmfulness: 
You don't know which cflags you are using, which directories are being passed
to gcc etc.]


Further comments/findings:

1.  Still one moderately serious warning:
libbacklight.c:81:2: warning: implicit declaration of function 'strtol'

2. libbacklight.h lacks a "header inclusion guard",
 i.e. something similar
to this:
#ifndef LIBBACKLIGHT_H
#define LIBBACKLIGHT_H
<actual contents>
#endif

3. Still no legal information in libbacklight.h (no copyright/license).

4. libbacklight.h includes <pciaccess.h>
I don't see libbacklight.h uses any external symbol which could originate from
<pciaccess.h> => makes me think this could be a bug.

However, should this #include be intentional, then libbacklight-devel would
need "Requires: libpciaccess-devel" + this dependency be reflected to
libbacklight.pc


Finally, a general remark: Please increment a package's rpm-revision each time
you replace it in reviews - It tremendously helps reviewers. TIA.

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