All of lore.kernel.org
 help / color / mirror / Atom feed
* [libdrm][PATCH 1/2] random: Use unsigned long for seed
@ 2015-02-09 21:39 Jan Vesely
  2015-02-09 21:39 ` [libdrm][PATCH 2/2] Fix gcc -Wextra warnings Jan Vesely
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Jan Vesely @ 2015-02-09 21:39 UTC (permalink / raw)
  To: dri-devel

This is more consistent with the rest, and avoids potential undefined
behavior (signed overflow) in drmRandom()

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
---
 xf86drmRandom.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/xf86drmRandom.c b/xf86drmRandom.c
index ecab9e2..a084b86 100644
--- a/xf86drmRandom.c
+++ b/xf86drmRandom.c
@@ -98,7 +98,7 @@ typedef struct RandomState {
     unsigned long q;		/* m div a */
     unsigned long r;		/* m mod a */
     unsigned long check;
-    long          seed;
+    unsigned long seed;
 } RandomState;
 
 #if RANDOM_MAIN
@@ -147,13 +147,13 @@ int drmRandomDestroy(void *state)
 unsigned long drmRandom(void *state)
 {
     RandomState   *s = (RandomState *)state;
-    long          hi;
-    long          lo;
+    unsigned long hi;
+    unsigned long lo;
 
     hi      = s->seed / s->q;
     lo      = s->seed % s->q;
     s->seed = s->a * lo - s->r * hi;
-    if (s->seed <= 0) s->seed += s->m;
+    if ((s->a * lo) <= (s->r * hi)) s->seed += s->m;
 
     return s->seed;
 }
@@ -166,7 +166,7 @@ double drmRandomDouble(void *state)
 }
 
 #if RANDOM_MAIN
-static void check_period(long seed)
+static void check_period(unsigned long seed)
 {
     unsigned long count = 0;
     unsigned long initial;
@@ -178,7 +178,7 @@ static void check_period(long seed)
     while (initial != drmRandom(state)) {
 	if (!++count) break;
     }
-    printf("With seed of %10ld, period = %10lu (0x%08lx)\n",
+    printf("With seed of %10lu, period = %10lu (0x%08lx)\n",
 	   seed, count, count);
     drmRandomDestroy(state);
 }
@@ -195,7 +195,7 @@ int main(void)
     }
     printf("After 10000 iterations: %lu (%lu expected): %s\n",
 	   rand, state->check,
-	   rand - state->check ? "*INCORRECT*" : "CORRECT");
+	   rand != state->check ? "*INCORRECT*" : "CORRECT");
     drmRandomDestroy(state);
 
     printf("Checking periods...\n");
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [libdrm][PATCH 2/2] Fix gcc -Wextra warnings
  2015-02-09 21:39 [libdrm][PATCH 1/2] random: Use unsigned long for seed Jan Vesely
@ 2015-02-09 21:39 ` Jan Vesely
  2015-02-09 23:12   ` Ian Romanick
  2015-02-09 23:32   ` Emil Velikov
  2015-02-09 23:11 ` [libdrm][PATCH 1/2] random: Use unsigned long for seed Ian Romanick
  2015-02-10  0:10 ` [libdrm][PATCH 3/2] Fix always true comparison Jan Vesely
  2 siblings, 2 replies; 16+ messages in thread
From: Jan Vesely @ 2015-02-09 21:39 UTC (permalink / raw)
  To: dri-devel

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH 1/2] random: Use unsigned long for seed
  2015-02-09 21:39 [libdrm][PATCH 1/2] random: Use unsigned long for seed Jan Vesely
  2015-02-09 21:39 ` [libdrm][PATCH 2/2] Fix gcc -Wextra warnings Jan Vesely
@ 2015-02-09 23:11 ` Ian Romanick
  2015-02-09 23:26   ` Jan Vesely
  2015-02-09 23:28   ` [libdrm][PATCH v2 " Jan Vesely
  2015-02-10  0:10 ` [libdrm][PATCH 3/2] Fix always true comparison Jan Vesely
  2 siblings, 2 replies; 16+ messages in thread
From: Ian Romanick @ 2015-02-09 23:11 UTC (permalink / raw)
  To: Jan Vesely, dri-devel

On 02/09/2015 01:39 PM, Jan Vesely wrote:
> This is more consistent with the rest, and avoids potential undefined
> behavior (signed overflow) in drmRandom()
> 
> Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
> ---
>  xf86drmRandom.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/xf86drmRandom.c b/xf86drmRandom.c
> index ecab9e2..a084b86 100644
> --- a/xf86drmRandom.c
> +++ b/xf86drmRandom.c
> @@ -98,7 +98,7 @@ typedef struct RandomState {
>      unsigned long q;		/* m div a */
>      unsigned long r;		/* m mod a */
>      unsigned long check;
> -    long          seed;
> +    unsigned long seed;
>  } RandomState;
>  
>  #if RANDOM_MAIN
> @@ -147,13 +147,13 @@ int drmRandomDestroy(void *state)
>  unsigned long drmRandom(void *state)
>  {
>      RandomState   *s = (RandomState *)state;
> -    long          hi;
> -    long          lo;
> +    unsigned long hi;
> +    unsigned long lo;
>  
>      hi      = s->seed / s->q;
>      lo      = s->seed % s->q;
>      s->seed = s->a * lo - s->r * hi;
> -    if (s->seed <= 0) s->seed += s->m;
> +    if ((s->a * lo) <= (s->r * hi)) s->seed += s->m;

This seems like an unrelated change.

>  
>      return s->seed;
>  }
> @@ -166,7 +166,7 @@ double drmRandomDouble(void *state)
>  }
>  
>  #if RANDOM_MAIN
> -static void check_period(long seed)
> +static void check_period(unsigned long seed)
>  {
>      unsigned long count = 0;
>      unsigned long initial;
> @@ -178,7 +178,7 @@ static void check_period(long seed)
>      while (initial != drmRandom(state)) {
>  	if (!++count) break;
>      }
> -    printf("With seed of %10ld, period = %10lu (0x%08lx)\n",
> +    printf("With seed of %10lu, period = %10lu (0x%08lx)\n",
>  	   seed, count, count);
>      drmRandomDestroy(state);
>  }
> @@ -195,7 +195,7 @@ int main(void)
>      }
>      printf("After 10000 iterations: %lu (%lu expected): %s\n",
>  	   rand, state->check,
> -	   rand - state->check ? "*INCORRECT*" : "CORRECT");
> +	   rand != state->check ? "*INCORRECT*" : "CORRECT");

So does this.

>      drmRandomDestroy(state);
>  
>      printf("Checking periods...\n");
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH 2/2] Fix gcc -Wextra warnings
  2015-02-09 21:39 ` [libdrm][PATCH 2/2] Fix gcc -Wextra warnings Jan Vesely
@ 2015-02-09 23:12   ` Ian Romanick
  2015-02-09 23:32   ` Emil Velikov
  1 sibling, 0 replies; 16+ messages in thread
From: Ian Romanick @ 2015-02-09 23:12 UTC (permalink / raw)
  To: Jan Vesely, dri-devel

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH 1/2] random: Use unsigned long for seed
  2015-02-09 23:11 ` [libdrm][PATCH 1/2] random: Use unsigned long for seed Ian Romanick
@ 2015-02-09 23:26   ` Jan Vesely
  2015-02-09 23:28   ` [libdrm][PATCH v2 " Jan Vesely
  1 sibling, 0 replies; 16+ messages in thread
From: Jan Vesely @ 2015-02-09 23:26 UTC (permalink / raw)
  To: Ian Romanick; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2655 bytes --]

On Mon, 2015-02-09 at 15:11 -0800, Ian Romanick wrote:
> On 02/09/2015 01:39 PM, Jan Vesely wrote:
> > This is more consistent with the rest, and avoids potential undefined
> > behavior (signed overflow) in drmRandom()
> > 
> > Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
> > ---
> >  xf86drmRandom.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/xf86drmRandom.c b/xf86drmRandom.c
> > index ecab9e2..a084b86 100644
> > --- a/xf86drmRandom.c
> > +++ b/xf86drmRandom.c
> > @@ -98,7 +98,7 @@ typedef struct RandomState {
> >      unsigned long q;		/* m div a */
> >      unsigned long r;		/* m mod a */
> >      unsigned long check;
> > -    long          seed;
> > +    unsigned long seed;
> >  } RandomState;
> >  
> >  #if RANDOM_MAIN
> > @@ -147,13 +147,13 @@ int drmRandomDestroy(void *state)
> >  unsigned long drmRandom(void *state)
> >  {
> >      RandomState   *s = (RandomState *)state;
> > -    long          hi;
> > -    long          lo;
> > +    unsigned long hi;
> > +    unsigned long lo;
> >  
> >      hi      = s->seed / s->q;
> >      lo      = s->seed % s->q;
> >      s->seed = s->a * lo - s->r * hi;
> > -    if (s->seed <= 0) s->seed += s->m;
> > +    if ((s->a * lo) <= (s->r * hi)) s->seed += s->m;
> 
> This seems like an unrelated change.

This change is necessary. since s->seed is unsigned now it cannot be <
0. also the result of s->seed op s->q is unsigned.

> 
> >  
> >      return s->seed;
> >  }
> > @@ -166,7 +166,7 @@ double drmRandomDouble(void *state)
> >  }
> >  
> >  #if RANDOM_MAIN
> > -static void check_period(long seed)
> > +static void check_period(unsigned long seed)
> >  {
> >      unsigned long count = 0;
> >      unsigned long initial;
> > @@ -178,7 +178,7 @@ static void check_period(long seed)
> >      while (initial != drmRandom(state)) {
> >  	if (!++count) break;
> >      }
> > -    printf("With seed of %10ld, period = %10lu (0x%08lx)\n",
> > +    printf("With seed of %10lu, period = %10lu (0x%08lx)\n",
> >  	   seed, count, count);
> >      drmRandomDestroy(state);
> >  }
> > @@ -195,7 +195,7 @@ int main(void)
> >      }
> >      printf("After 10000 iterations: %lu (%lu expected): %s\n",
> >  	   rand, state->check,
> > -	   rand - state->check ? "*INCORRECT*" : "CORRECT");
> > +	   rand != state->check ? "*INCORRECT*" : "CORRECT");
> 
> So does this.

I'll drop this one in v2, it shouldn't have been included

> 
> >      drmRandomDestroy(state);
> >  
> >      printf("Checking periods...\n");
> > 
> 

-- 
Jan Vesely <jan.vesely@rutgers.edu>

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [libdrm][PATCH v2 1/2] random: Use unsigned long for seed
  2015-02-09 23:11 ` [libdrm][PATCH 1/2] random: Use unsigned long for seed Ian Romanick
  2015-02-09 23:26   ` Jan Vesely
@ 2015-02-09 23:28   ` Jan Vesely
  2015-02-10 18:21     ` Ian Romanick
  1 sibling, 1 reply; 16+ messages in thread
From: Jan Vesely @ 2015-02-09 23:28 UTC (permalink / raw)
  To: dri-devel, Ian Romanick

v2: Remove unrelated change in main()

This is more consistent with the rest, and avoids potential undefined
behavior (signed overflow) ind drmRandom()

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
---
 xf86drmRandom.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xf86drmRandom.c b/xf86drmRandom.c
index ecab9e2..94922ad 100644
--- a/xf86drmRandom.c
+++ b/xf86drmRandom.c
@@ -98,7 +98,7 @@ typedef struct RandomState {
     unsigned long q;		/* m div a */
     unsigned long r;		/* m mod a */
     unsigned long check;
-    long          seed;
+    unsigned long seed;
 } RandomState;
 
 #if RANDOM_MAIN
@@ -147,13 +147,13 @@ int drmRandomDestroy(void *state)
 unsigned long drmRandom(void *state)
 {
     RandomState   *s = (RandomState *)state;
-    long          hi;
-    long          lo;
+    unsigned long hi;
+    unsigned long lo;
 
     hi      = s->seed / s->q;
     lo      = s->seed % s->q;
     s->seed = s->a * lo - s->r * hi;
-    if (s->seed <= 0) s->seed += s->m;
+    if ((s->a * lo) <= (s->r * hi)) s->seed += s->m;
 
     return s->seed;
 }
@@ -166,7 +166,7 @@ double drmRandomDouble(void *state)
 }
 
 #if RANDOM_MAIN
-static void check_period(long seed)
+static void check_period(unsigned long seed)
 {
     unsigned long count = 0;
     unsigned long initial;
@@ -178,7 +178,7 @@ static void check_period(long seed)
     while (initial != drmRandom(state)) {
 	if (!++count) break;
     }
-    printf("With seed of %10ld, period = %10lu (0x%08lx)\n",
+    printf("With seed of %10lu, period = %10lu (0x%08lx)\n",
 	   seed, count, count);
     drmRandomDestroy(state);
 }
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH 2/2] Fix gcc -Wextra warnings
  2015-02-09 21:39 ` [libdrm][PATCH 2/2] Fix gcc -Wextra warnings Jan Vesely
  2015-02-09 23:12   ` Ian Romanick
@ 2015-02-09 23:32   ` Emil Velikov
  2015-02-10  0:02     ` Jan Vesely
  1 sibling, 1 reply; 16+ messages in thread
From: Emil Velikov @ 2015-02-09 23:32 UTC (permalink / raw)
  To: Jan Vesely; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH 2/2] Fix gcc -Wextra warnings
  2015-02-09 23:32   ` Emil Velikov
@ 2015-02-10  0:02     ` Jan Vesely
  2015-02-10  0:27       ` Emil Velikov
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Vesely @ 2015-02-10  0:02 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 911 bytes --]

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

-- 
Jan Vesely <jan.vesely@rutgers.edu>

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [libdrm][PATCH 3/2] Fix always true comparison.
  2015-02-09 21:39 [libdrm][PATCH 1/2] random: Use unsigned long for seed Jan Vesely
  2015-02-09 21:39 ` [libdrm][PATCH 2/2] Fix gcc -Wextra warnings Jan Vesely
  2015-02-09 23:11 ` [libdrm][PATCH 1/2] random: Use unsigned long for seed Ian Romanick
@ 2015-02-10  0:10 ` Jan Vesely
  2015-02-25 17:11   ` Jan Vesely
  2 siblings, 1 reply; 16+ messages in thread
From: Jan Vesely @ 2015-02-10  0:10 UTC (permalink / raw)
  To: dri-devel, Ian Romanick, Emil Velikov

The only user I found is xserver, it can return -1 under certain conditions.
So check for -1 explicitly.

Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
---

I could not find whether it's actually legal to return encoded negative values
in get_perm. This is a quick fix to detect the one case that I found.

 xf86drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xf86drm.c b/xf86drm.c
index fb673b5..8e54ac9 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -335,7 +335,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
 	drm_server_info->get_perms(&serv_group, &serv_mode);
 	devmode  = serv_mode ? serv_mode : DRM_DEV_MODE;
 	devmode &= ~(S_IXUSR|S_IXGRP|S_IXOTH);
-	group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
+	group = (serv_group != ~0U) ? serv_group : DRM_DEV_GID;
     }
 
 #if !defined(UDEV)
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH 2/2] Fix gcc -Wextra warnings
  2015-02-10  0:02     ` Jan Vesely
@ 2015-02-10  0:27       ` Emil Velikov
  2015-02-10 21:37         ` Jan Vesely
  0 siblings, 1 reply; 16+ messages in thread
From: Emil Velikov @ 2015-02-10  0:27 UTC (permalink / raw)
  To: Jan Vesely; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH v2 1/2] random: Use unsigned long for seed
  2015-02-09 23:28   ` [libdrm][PATCH v2 " Jan Vesely
@ 2015-02-10 18:21     ` Ian Romanick
  0 siblings, 0 replies; 16+ messages in thread
From: Ian Romanick @ 2015-02-10 18:21 UTC (permalink / raw)
  To: Jan Vesely, dri-devel

This patch is

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

On 02/09/2015 03:28 PM, Jan Vesely wrote:
> v2: Remove unrelated change in main()
> 
> This is more consistent with the rest, and avoids potential undefined
> behavior (signed overflow) ind drmRandom()
> 
> Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
> ---
>  xf86drmRandom.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/xf86drmRandom.c b/xf86drmRandom.c
> index ecab9e2..94922ad 100644
> --- a/xf86drmRandom.c
> +++ b/xf86drmRandom.c
> @@ -98,7 +98,7 @@ typedef struct RandomState {
>      unsigned long q;		/* m div a */
>      unsigned long r;		/* m mod a */
>      unsigned long check;
> -    long          seed;
> +    unsigned long seed;
>  } RandomState;
>  
>  #if RANDOM_MAIN
> @@ -147,13 +147,13 @@ int drmRandomDestroy(void *state)
>  unsigned long drmRandom(void *state)
>  {
>      RandomState   *s = (RandomState *)state;
> -    long          hi;
> -    long          lo;
> +    unsigned long hi;
> +    unsigned long lo;
>  
>      hi      = s->seed / s->q;
>      lo      = s->seed % s->q;
>      s->seed = s->a * lo - s->r * hi;
> -    if (s->seed <= 0) s->seed += s->m;
> +    if ((s->a * lo) <= (s->r * hi)) s->seed += s->m;
>  
>      return s->seed;
>  }
> @@ -166,7 +166,7 @@ double drmRandomDouble(void *state)
>  }
>  
>  #if RANDOM_MAIN
> -static void check_period(long seed)
> +static void check_period(unsigned long seed)
>  {
>      unsigned long count = 0;
>      unsigned long initial;
> @@ -178,7 +178,7 @@ static void check_period(long seed)
>      while (initial != drmRandom(state)) {
>  	if (!++count) break;
>      }
> -    printf("With seed of %10ld, period = %10lu (0x%08lx)\n",
> +    printf("With seed of %10lu, period = %10lu (0x%08lx)\n",
>  	   seed, count, count);
>      drmRandomDestroy(state);
>  }
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH 2/2] Fix gcc -Wextra warnings
  2015-02-10  0:27       ` Emil Velikov
@ 2015-02-10 21:37         ` Jan Vesely
  2015-02-10 22:55           ` Emil Velikov
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Vesely @ 2015-02-10 21:37 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2481 bytes --]

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

-- 
Jan Vesely <jan.vesely@rutgers.edu>

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH 2/2] Fix gcc -Wextra warnings
  2015-02-10 21:37         ` Jan Vesely
@ 2015-02-10 22:55           ` Emil Velikov
  0 siblings, 0 replies; 16+ messages in thread
From: Emil Velikov @ 2015-02-10 22:55 UTC (permalink / raw)
  To: Jan Vesely; +Cc: emil.l.velikov, ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH 3/2] Fix always true comparison.
  2015-02-10  0:10 ` [libdrm][PATCH 3/2] Fix always true comparison Jan Vesely
@ 2015-02-25 17:11   ` Jan Vesely
  2015-02-25 18:41     ` Emil Velikov
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Vesely @ 2015-02-25 17:11 UTC (permalink / raw)
  To: dri-devel; +Cc: Emil Velikov


[-- Attachment #1.1: Type: text/plain, Size: 1032 bytes --]

gentle ping


On Mon, 2015-02-09 at 19:10 -0500, Jan Vesely wrote:
> The only user I found is xserver, it can return -1 under certain conditions.
> So check for -1 explicitly.
> 
> Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
> ---
> 
> I could not find whether it's actually legal to return encoded negative values
> in get_perm. This is a quick fix to detect the one case that I found.
> 
>  xf86drm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index fb673b5..8e54ac9 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -335,7 +335,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
>  	drm_server_info->get_perms(&serv_group, &serv_mode);
>  	devmode  = serv_mode ? serv_mode : DRM_DEV_MODE;
>  	devmode &= ~(S_IXUSR|S_IXGRP|S_IXOTH);
> -	group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
> +	group = (serv_group != ~0U) ? serv_group : DRM_DEV_GID;
>      }
>  
>  #if !defined(UDEV)

-- 
Jan Vesely <jan.vesely@rutgers.edu>

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH 3/2] Fix always true comparison.
  2015-02-25 17:11   ` Jan Vesely
@ 2015-02-25 18:41     ` Emil Velikov
  2015-03-02 20:01       ` Jan Vesely
  0 siblings, 1 reply; 16+ messages in thread
From: Emil Velikov @ 2015-02-25 18:41 UTC (permalink / raw)
  To: Jan Vesely, dri-devel; +Cc: emil.l.velikov

On 25/02/15 17:11, Jan Vesely wrote:
> gentle ping
> 
Afaics it's very had to get in this code nowadays - drm_server_info is
set only via the legacy (?) function drmSetServerInfo. With the latter
only(?) used by the xserver when working with dri1 modules. So testing
this is likely to be very painful :-(

This code hasn't changed since before 2007, so I doubt there are many
people that know the details about it, so we might as well leave it for
now ?

-Emil

> 
> On Mon, 2015-02-09 at 19:10 -0500, Jan Vesely wrote:
>> The only user I found is xserver, it can return -1 under certain conditions.
>> So check for -1 explicitly.
>>
>> Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
>> ---
>>
>> I could not find whether it's actually legal to return encoded negative values
>> in get_perm. This is a quick fix to detect the one case that I found.
>>
>>  xf86drm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xf86drm.c b/xf86drm.c
>> index fb673b5..8e54ac9 100644
>> --- a/xf86drm.c
>> +++ b/xf86drm.c
>> @@ -335,7 +335,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
>>  	drm_server_info->get_perms(&serv_group, &serv_mode);
>>  	devmode  = serv_mode ? serv_mode : DRM_DEV_MODE;
>>  	devmode &= ~(S_IXUSR|S_IXGRP|S_IXOTH);
>> -	group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
>> +	group = (serv_group != ~0U) ? serv_group : DRM_DEV_GID;
>>      }
>>  
>>  #if !defined(UDEV)
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [libdrm][PATCH 3/2] Fix always true comparison.
  2015-02-25 18:41     ` Emil Velikov
@ 2015-03-02 20:01       ` Jan Vesely
  0 siblings, 0 replies; 16+ messages in thread
From: Jan Vesely @ 2015-03-02 20:01 UTC (permalink / raw)
  To: Emil Velikov; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1783 bytes --]

On Wed, 2015-02-25 at 18:41 +0000, Emil Velikov wrote:
> On 25/02/15 17:11, Jan Vesely wrote:
> > gentle ping
> > 
> Afaics it's very had to get in this code nowadays - drm_server_info is
> set only via the legacy (?) function drmSetServerInfo. With the latter
> only(?) used by the xserver when working with dri1 modules. So testing
> this is likely to be very painful :-(
> 
> This code hasn't changed since before 2007, so I doubt there are many
> people that know the details about it, so we might as well leave it for
> now ?

fair enough. the warning fixes push it to non-libudev side, so it's not
going to bug me on every build.

jan

> 
> -Emil
> 
> > 
> > On Mon, 2015-02-09 at 19:10 -0500, Jan Vesely wrote:
> >> The only user I found is xserver, it can return -1 under certain conditions.
> >> So check for -1 explicitly.
> >>
> >> Signed-off-by: Jan Vesely <jan.vesely@rutgers.edu>
> >> ---
> >>
> >> I could not find whether it's actually legal to return encoded negative values
> >> in get_perm. This is a quick fix to detect the one case that I found.
> >>
> >>  xf86drm.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/xf86drm.c b/xf86drm.c
> >> index fb673b5..8e54ac9 100644
> >> --- a/xf86drm.c
> >> +++ b/xf86drm.c
> >> @@ -335,7 +335,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
> >>  	drm_server_info->get_perms(&serv_group, &serv_mode);
> >>  	devmode  = serv_mode ? serv_mode : DRM_DEV_MODE;
> >>  	devmode &= ~(S_IXUSR|S_IXGRP|S_IXOTH);
> >> -	group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
> >> +	group = (serv_group != ~0U) ? serv_group : DRM_DEV_GID;
> >>      }
> >>  
> >>  #if !defined(UDEV)
> > 
> 

-- 
Jan Vesely <jan.vesely@rutgers.edu>

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-03-02 20:01 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-09 21:39 [libdrm][PATCH 1/2] random: Use unsigned long for seed Jan Vesely
2015-02-09 21:39 ` [libdrm][PATCH 2/2] Fix gcc -Wextra warnings Jan Vesely
2015-02-09 23:12   ` Ian Romanick
2015-02-09 23:32   ` Emil Velikov
2015-02-10  0:02     ` Jan Vesely
2015-02-10  0:27       ` Emil Velikov
2015-02-10 21:37         ` Jan Vesely
2015-02-10 22:55           ` Emil Velikov
2015-02-09 23:11 ` [libdrm][PATCH 1/2] random: Use unsigned long for seed Ian Romanick
2015-02-09 23:26   ` Jan Vesely
2015-02-09 23:28   ` [libdrm][PATCH v2 " Jan Vesely
2015-02-10 18:21     ` Ian Romanick
2015-02-10  0:10 ` [libdrm][PATCH 3/2] Fix always true comparison Jan Vesely
2015-02-25 17:11   ` Jan Vesely
2015-02-25 18:41     ` Emil Velikov
2015-03-02 20:01       ` Jan Vesely

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.