diff mbox

[libdrm,2/2] Fix gcc -Wextra warnings

Message ID 1423517995-28251-2-git-send-email-jan.vesely@rutgers.edu (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Vesely Feb. 9, 2015, 9:39 p.m. UTC
Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
---
 tests/vbltest/vbltest.c | 3 ++-
 xf86drm.c               | 4 ++--
 xf86drmMode.c           | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

Comments

Ian Romanick Feb. 9, 2015, 11:12 p.m. UTC | #1
This patch is

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

On 02/09/2015 01:39 PM, Jan Vesely wrote:
> Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
> ---
>  tests/vbltest/vbltest.c | 3 ++-
>  xf86drm.c               | 4 ++--
>  xf86drmMode.c           | 2 +-
>  3 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/vbltest/vbltest.c b/tests/vbltest/vbltest.c
> index cdc1ef6..6e13c57 100644
> --- a/tests/vbltest/vbltest.c
> +++ b/tests/vbltest/vbltest.c
> @@ -104,7 +104,8 @@ static void usage(char *name)
>  
>  int main(int argc, char **argv)
>  {
> -	int i, c, fd, ret;
> +	unsigned i;
> +	int c, fd, ret;
>  	char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "exynos", "omapdrm", "tilcdc", "msm", "tegra" };
>  	drmVBlank vbl;
>  	drmEventContext evctx;
> diff --git a/xf86drm.c b/xf86drm.c
> index 345325a..fb673b5 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -303,7 +303,7 @@ static int chown_check_return(const char *path, uid_t owner, gid_t group)
>   * special file node with the major and minor numbers specified by \p dev and
>   * parent directory if necessary and was called by root.
>   */
> -static int drmOpenDevice(long dev, int minor, int type)
> +static int drmOpenDevice(dev_t dev, int minor, int type)
>  {
>      stat_t          st;
>      const char      *dev_name;
> @@ -2213,7 +2213,7 @@ int drmGetClient(int fd, int idx, int *auth, int *pid, int *uid,
>  int drmGetStats(int fd, drmStatsT *stats)
>  {
>      drm_stats_t s;
> -    int         i;
> +    unsigned    i;
>  
>      if (drmIoctl(fd, DRM_IOCTL_GET_STATS, &s))
>  	return -errno;
> diff --git a/xf86drmMode.c b/xf86drmMode.c
> index 60ce369..e3e599b 100644
> --- a/xf86drmMode.c
> +++ b/xf86drmMode.c
> @@ -854,7 +854,7 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx)
>  	len = read(fd, buffer, sizeof buffer);
>  	if (len == 0)
>  		return 0;
> -	if (len < sizeof *e)
> +	if (len < (int)sizeof *e)
>  		return -1;
>  
>  	i = 0;
>
Emil Velikov Feb. 9, 2015, 11:32 p.m. UTC | #2
On 9 February 2015 at 21:39, Jan Vesely <jan.vesely@rutgers.edu> wrote:
> Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
Nice one Jan. I've sent similar fixes for drmOpenDevice and
drmGetStats a few days ago.

Considering you drop the last hunk that Ian spotted both patches are
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

The strange part is that the normal Linux build does not show even a
single warning, despite the -Wextra and -Wsign-compare flags from
configure.ac. Perhaps my gcc does not like libdrm for some reason :P

-Emil
Jan Vesely Feb. 10, 2015, 12:02 a.m. UTC | #3
On Mon, 2015-02-09 at 23:32 +0000, Emil Velikov wrote:
> On 9 February 2015 at 21:39, Jan Vesely <jan.vesely@rutgers.edu> wrote:
> > Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
> Nice one Jan. I've sent similar fixes for drmOpenDevice and
> drmGetStats a few days ago.
> 
> Considering you drop the last hunk that Ian spotted both patches are
> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

Thanks, I sent v2 of that patch few minutes ago.

I think your 4/6 and 5/6 overlap with this one. Should I go ahead or do
you plan to push yours?

> 
> The strange part is that the normal Linux build does not show even a
> single warning, despite the -Wextra and -Wsign-compare flags from
> configure.ac. Perhaps my gcc does not like libdrm for some reason :P

I think I just used CFLAGS= during configure, and it worked
jan

> 
> -Emil
Emil Velikov Feb. 10, 2015, 12:27 a.m. UTC | #4
On 10 February 2015 at 00:02, Jan Vesely <jan.vesely@rutgers.edu> wrote:
> On Mon, 2015-02-09 at 23:32 +0000, Emil Velikov wrote:
>> On 9 February 2015 at 21:39, Jan Vesely <jan.vesely@rutgers.edu> wrote:
>> > Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
>> Nice one Jan. I've sent similar fixes for drmOpenDevice and
>> drmGetStats a few days ago.
>>
>> Considering you drop the last hunk that Ian spotted both patches are
>> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
>
> Thanks, I sent v2 of that patch few minutes ago.
>
> I think your 4/6 and 5/6 overlap with this one. Should I go ahead or do
> you plan to push yours?
>
I would go with your series - it handles more cases, plus already has
move reviews :-P
If you feel like looking through some of my series that would be appreciated.

>>
>> The strange part is that the normal Linux build does not show even a
>> single warning, despite the -Wextra and -Wsign-compare flags from
>> configure.ac. Perhaps my gcc does not like libdrm for some reason :P
>
> I think I just used CFLAGS= during configure, and it worked
> jan
>
Yes the issue was that we're not using WARN_CFLAGS in every Makefile in libdrm.
Namely the top one and most of the tests.

-Emil
Jan Vesely Feb. 10, 2015, 9:37 p.m. UTC | #5
On Tue, 2015-02-10 at 00:27 +0000, Emil Velikov wrote:
> On 10 February 2015 at 00:02, Jan Vesely <jan.vesely@rutgers.edu> wrote:
> > On Mon, 2015-02-09 at 23:32 +0000, Emil Velikov wrote:
> >> On 9 February 2015 at 21:39, Jan Vesely <jan.vesely@rutgers.edu> wrote:
> >> > Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
> >> Nice one Jan. I've sent similar fixes for drmOpenDevice and
> >> drmGetStats a few days ago.
> >>
> >> Considering you drop the last hunk that Ian spotted both patches are
> >> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
> >
> > Thanks, I sent v2 of that patch few minutes ago.
> >
> > I think your 4/6 and 5/6 overlap with this one. Should I go ahead or do
> > you plan to push yours?
> >
> I would go with your series - it handles more cases, plus already has
> move reviews :-P
> If you feel like looking through some of my series that would be appreciated.

I wasn't subscribed to the list so I can't reply to those emails (don't
know the message-ids).
I looked at the series from Jan 29th [0]. 

1/6[1], there is no tests/util directory, I guess it depends on
Thierry's series? since it hasn't landed yet does it make sense to
squash it there (like your 04.1/11 SQUASH: tests: misc cleanups) ?

2/6[2], also does not apply cleanly (needs Thierry's 5/11), if you want
to push a version rebased on master you can add

Reviewed-by: Jan Vesely <jan.vesely@rutgers.edu>
to that one

4/6 and 5/6 were superseded, and I don't know enough about android to
look at the other two, but

6/6 looks trivial enough
Acked-by: Jan Vesely <jan.vesely@rutgers.edu>

with a small nit:
Why keep two assignments to LOCAL_SHARED_LIBRARIES in intel/Android.mk ?

regards,
jan


[0]http://lists.freedesktop.org/archives/dri-devel/2015-January/076456.html
[1]http://lists.freedesktop.org/archives/dri-devel/2015-January/076457.html
[2]http://lists.freedesktop.org/archives/dri-devel/2015-January/076458.html


> 
> >>
> >> The strange part is that the normal Linux build does not show even a
> >> single warning, despite the -Wextra and -Wsign-compare flags from
> >> configure.ac. Perhaps my gcc does not like libdrm for some reason :P
> >
> > I think I just used CFLAGS= during configure, and it worked
> > jan
> >
> Yes the issue was that we're not using WARN_CFLAGS in every Makefile in libdrm.
> Namely the top one and most of the tests.
> 
> -Emil
Emil Velikov Feb. 10, 2015, 10:55 p.m. UTC | #6
On 10/02/15 21:37, Jan Vesely wrote:
> On Tue, 2015-02-10 at 00:27 +0000, Emil Velikov wrote:
>> On 10 February 2015 at 00:02, Jan Vesely <jan.vesely@rutgers.edu> wrote:
>>> On Mon, 2015-02-09 at 23:32 +0000, Emil Velikov wrote:
>>>> On 9 February 2015 at 21:39, Jan Vesely <jan.vesely@rutgers.edu> wrote:
>>>>> Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
>>>> Nice one Jan. I've sent similar fixes for drmOpenDevice and
>>>> drmGetStats a few days ago.
>>>>
>>>> Considering you drop the last hunk that Ian spotted both patches are
>>>> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
>>>
>>> Thanks, I sent v2 of that patch few minutes ago.
>>>
>>> I think your 4/6 and 5/6 overlap with this one. Should I go ahead or do
>>> you plan to push yours?
>>>
>> I would go with your series - it handles more cases, plus already has
>> move reviews :-P
>> If you feel like looking through some of my series that would be appreciated.
> 
> I wasn't subscribed to the list so I can't reply to those emails (don't
> know the message-ids).
> I looked at the series from Jan 29th [0]. 
> 
> 1/6[1], there is no tests/util directory, I guess it depends on
> Thierry's series? since it hasn't landed yet does it make sense to
> squash it there (like your 04.1/11 SQUASH: tests: misc cleanups) ?
> 
> 2/6[2], also does not apply cleanly (needs Thierry's 5/11), if you want
> to push a version rebased on master you can add
> 
> Reviewed-by: Jan Vesely <jan.vesely@rutgers.edu>
> to that one
> 
I'll these on hold and revive as Theirry's series lands.

> 4/6 and 5/6 were superseded, and I don't know enough about android to
> look at the other two, but
> 
> 6/6 looks trivial enough
> Acked-by: Jan Vesely <jan.vesely@rutgers.edu>
> 
> with a small nit:
> Why keep two assignments to LOCAL_SHARED_LIBRARIES in intel/Android.mk ?
> 
Good catch. I'm assuming that (a) either I messed up at cherry-picking
the patch or (b) git got confused as their three does not have
libpciaccess in the list.

Thanks
Emil
diff mbox

Patch

diff --git a/tests/vbltest/vbltest.c b/tests/vbltest/vbltest.c
index cdc1ef6..6e13c57 100644
--- a/tests/vbltest/vbltest.c
+++ b/tests/vbltest/vbltest.c
@@ -104,7 +104,8 @@  static void usage(char *name)
 
 int main(int argc, char **argv)
 {
-	int i, c, fd, ret;
+	unsigned i;
+	int c, fd, ret;
 	char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "exynos", "omapdrm", "tilcdc", "msm", "tegra" };
 	drmVBlank vbl;
 	drmEventContext evctx;
diff --git a/xf86drm.c b/xf86drm.c
index 345325a..fb673b5 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -303,7 +303,7 @@  static int chown_check_return(const char *path, uid_t owner, gid_t group)
  * special file node with the major and minor numbers specified by \p dev and
  * parent directory if necessary and was called by root.
  */
-static int drmOpenDevice(long dev, int minor, int type)
+static int drmOpenDevice(dev_t dev, int minor, int type)
 {
     stat_t          st;
     const char      *dev_name;
@@ -2213,7 +2213,7 @@  int drmGetClient(int fd, int idx, int *auth, int *pid, int *uid,
 int drmGetStats(int fd, drmStatsT *stats)
 {
     drm_stats_t s;
-    int         i;
+    unsigned    i;
 
     if (drmIoctl(fd, DRM_IOCTL_GET_STATS, &s))
 	return -errno;
diff --git a/xf86drmMode.c b/xf86drmMode.c
index 60ce369..e3e599b 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -854,7 +854,7 @@  int drmHandleEvent(int fd, drmEventContextPtr evctx)
 	len = read(fd, buffer, sizeof buffer);
 	if (len == 0)
 		return 0;
-	if (len < sizeof *e)
+	if (len < (int)sizeof *e)
 		return -1;
 
 	i = 0;