From: Pauli N. <su...@gm...> - 2009-07-03 23:19:06
|
I did try to compile libdrm with "-pedantic -Wall -Werror" but there is a lot of warnings generated. This patch set fixes compilation of libdrm/xf86drm.c. I also made patch to add configuration option to make it easy for developers to use strict compilation with all warnings enabled as errors. I'm not sure that all patches are the best aproach to fix the warnings so comments are very welcome. Pauli |
From: Pauli N. <su...@gm...> - 2009-07-03 23:19:19
|
--- shared-core/drm.h | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/shared-core/drm.h b/shared-core/drm.h index 7758af4..42a6c23 100644 --- a/shared-core/drm.h +++ b/shared-core/drm.h @@ -855,7 +855,6 @@ typedef struct drm_set_version drm_set_version_t; typedef struct drm_fence_arg drm_fence_arg_t; typedef struct drm_mm_type_arg drm_mm_type_arg_t; typedef struct drm_mm_init_arg drm_mm_init_arg_t; -typedef enum drm_bo_type drm_bo_type_t; #endif #endif -- 1.6.3.1 |
From: Pauli N. <su...@gm...> - 2009-07-03 23:19:20
|
--- libdrm/xf86drm.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/libdrm/xf86drm.h b/libdrm/xf86drm.h index c1d173c..a7f9763 100644 --- a/libdrm/xf86drm.h +++ b/libdrm/xf86drm.h @@ -37,6 +37,7 @@ #include <stdarg.h> #include <sys/types.h> #include <stdint.h> +#include <strings.h> #include <drm.h> /* Defaults, if nothing set in xf86config */ -- 1.6.3.1 |
From: Pauli N. <su...@gm...> - 2009-07-03 23:19:21
|
--- libdrm/xf86drm.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libdrm/xf86drm.c b/libdrm/xf86drm.c index 55df19a..1e6eb7a 100644 --- a/libdrm/xf86drm.c +++ b/libdrm/xf86drm.c @@ -240,22 +240,22 @@ static int drmMatchBusID(const char *id1, const char *id2) /* Try to match old/new-style PCI bus IDs. */ if (strncasecmp(id1, "pci", 3) == 0) { - int o1, b1, d1, f1; - int o2, b2, d2, f2; + unsigned int o1, b1, d1, f1; + unsigned int o2, b2, d2, f2; int ret; - ret = sscanf(id1, "pci:%04x:%02x:%02x.%d", &o1, &b1, &d1, &f1); + ret = sscanf(id1, "pci:%04x:%02x:%02x.%u", &o1, &b1, &d1, &f1); if (ret != 4) { o1 = 0; - ret = sscanf(id1, "PCI:%d:%d:%d", &b1, &d1, &f1); + ret = sscanf(id1, "PCI:%u:%u:%u", &b1, &d1, &f1); if (ret != 3) return 0; } - ret = sscanf(id2, "pci:%04x:%02x:%02x.%d", &o2, &b2, &d2, &f2); + ret = sscanf(id2, "pci:%04x:%02x:%02x.%u", &o2, &b2, &d2, &f2); if (ret != 4) { o2 = 0; - ret = sscanf(id2, "PCI:%d:%d:%d", &b2, &d2, &f2); + ret = sscanf(id2, "PCI:%u:%u:%u", &b2, &d2, &f2); if (ret != 3) return 0; } -- 1.6.3.1 |
From: Pauli N. <su...@gm...> - 2009-07-03 23:19:19
|
--- libdrm/xf86drm.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/libdrm/xf86drm.c b/libdrm/xf86drm.c index 1e6eb7a..7b05386 100644 --- a/libdrm/xf86drm.c +++ b/libdrm/xf86drm.c @@ -34,6 +34,7 @@ #ifdef HAVE_CONFIG_H # include <config.h> #endif +#define _XOPEN_SOURCE 500 #include <stdio.h> #include <stdlib.h> #include <unistd.h> -- 1.6.3.1 |
From: Ian R. <id...@fr...> - 2009-07-06 16:47:34
|
On Sat, Jul 04, 2009 at 02:18:52AM +0300, Pauli Nieminen wrote: > --- > libdrm/xf86drm.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/libdrm/xf86drm.c b/libdrm/xf86drm.c > index 1e6eb7a..7b05386 100644 > --- a/libdrm/xf86drm.c > +++ b/libdrm/xf86drm.c > @@ -34,6 +34,7 @@ > #ifdef HAVE_CONFIG_H > # include <config.h> > #endif > +#define _XOPEN_SOURCE 500 > #include <stdio.h> > #include <stdlib.h> > #include <unistd.h> I don't think setting this directly in a source file is the correct way. Here's an alterate patch the puts _XOPEN_SOURCE and _GNU_SOURCE in config.h via configure.ac. Comments? commit 716e4ad01bac6fdb6e3f433300df3cf50116d3b1 Author: Ian Romanick <ian...@in...> Date: Mon Jul 6 09:41:56 2009 -0700 libdrm: Set _XOPEN_SOURCE and _GNU_SOURCE Several POSIX extensions are used in the libdrm code (e.g., mknod and ffs). Set _XOPEN_SOURCE to 500 and set _GNU_SOURCE to non-zero to ensure that prototypes for these functions are available. This is done in configure.ac that checks can eventually be added to only enable these on operating system that support them. This is based on a patch by Pauli Nieminen. Thanks. Signed-off-by: Ian Romanick <ian...@in...> diff --git a/configure.ac b/configure.ac index be2fbbf..b54e64f 100644 --- a/configure.ac +++ b/configure.ac @@ -145,6 +145,8 @@ if test "x$HAVE_LIBUDEV" = xyes; then fi AM_CONDITIONAL(HAVE_LIBUDEV, [test "x$HAVE_LIBUDEV" = xyes]) +AC_DEFINE(_XOPEN_SOURCE, 500, [XOpen source]) +AC_DEFINE(_GNU_SOURCE, 1, [GNU source]) AC_SUBST(WARN_CFLAGS) AC_OUTPUT([ |
From: Julien C. <jcr...@de...> - 2009-07-06 16:59:20
|
On Mon, Jul 6, 2009 at 09:46:40 -0700, Ian Romanick wrote: > Here's an alterate patch the puts _XOPEN_SOURCE and _GNU_SOURCE in config.h > via configure.ac. Comments? > maybe use AC_USE_SYSTEM_EXTENSIONS instead? Cheers, Julien |
From: Pauli N. <su...@gm...> - 2009-07-06 20:00:37
|
Configure setting it sounds good to me. _GNU_SOURCE implies XOPEN_SOURCE set to 600 (That should include everything from the latest standard if I'm not mistaken) I did set 500 because it was enough for XSH api. On Mon, Jul 6, 2009 at 7:59 PM, Julien Cristau<jcr...@de...> wrote: > On Mon, Jul 6, 2009 at 09:46:40 -0700, Ian Romanick wrote: > >> Here's an alterate patch the puts _XOPEN_SOURCE and _GNU_SOURCE in config.h >> via configure.ac. Comments? >> > maybe use AC_USE_SYSTEM_EXTENSIONS instead? > > Cheers, > Julien > |
From: Alan C. <Alan.Coopersmith@Sun.COM> - 2009-07-06 20:16:55
|
Julien Cristau wrote: > On Mon, Jul 6, 2009 at 09:46:40 -0700, Ian Romanick wrote: > >> Here's an alterate patch the puts _XOPEN_SOURCE and _GNU_SOURCE in config.h >> via configure.ac. Comments? >> > maybe use AC_USE_SYSTEM_EXTENSIONS instead? Yes please - on some systems (Solaris for instance), setting _XOPEN_SOURCE reduces the available API to just the X/Open standards, instead of extending it as it does on Linux - the AC_USE_SYSTEM_EXTENSIONS knows how to set this correctly for both. -- -Alan Coopersmith- ala...@su... Sun Microsystems, Inc. - X Window System Engineering |
From: Ian R. <id...@fr...> - 2009-07-06 21:36:27
|
On Mon, Jul 06, 2009 at 01:14:14PM -0700, Alan Coopersmith wrote: > Julien Cristau wrote: > > On Mon, Jul 6, 2009 at 09:46:40 -0700, Ian Romanick wrote: > > > >> Here's an alterate patch the puts _XOPEN_SOURCE and _GNU_SOURCE in config.h > >> via configure.ac. Comments? > >> > > maybe use AC_USE_SYSTEM_EXTENSIONS instead? > > Yes please - on some systems (Solaris for instance), setting _XOPEN_SOURCE > reduces the available API to just the X/Open standards, instead of extending > it as it does on Linux - the AC_USE_SYSTEM_EXTENSIONS knows how to set this > correctly for both. Grr. So, I did this. The first time I run autogen.sh I get the following error: configure.ac:34: error: possibly undefined macro: AC_USE_SYSTEM_MACROS If this token and others are legitimate, please use m4_pattern_allow. See the Autoconf documentation. Running configure by hand after that works fine. Why would I need to use m4_pattern_allow for an autoconf built-in test? |
From: Ian R. <id...@fr...> - 2009-07-06 22:39:48
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Ian Romanick wrote: > On Mon, Jul 06, 2009 at 01:14:14PM -0700, Alan Coopersmith wrote: >> Julien Cristau wrote: >>> On Mon, Jul 6, 2009 at 09:46:40 -0700, Ian Romanick wrote: >>> >>>> Here's an alterate patch the puts _XOPEN_SOURCE and _GNU_SOURCE in config.h >>>> via configure.ac. Comments? >>>> >>> maybe use AC_USE_SYSTEM_EXTENSIONS instead? >> Yes please - on some systems (Solaris for instance), setting _XOPEN_SOURCE >> reduces the available API to just the X/Open standards, instead of extending >> it as it does on Linux - the AC_USE_SYSTEM_EXTENSIONS knows how to set this >> correctly for both. > > Grr. So, I did this. The first time I run autogen.sh I get the following > error: > > configure.ac:34: error: possibly undefined macro: AC_USE_SYSTEM_MACROS > If this token and others are legitimate, please use m4_pattern_allow. > See the Autoconf documentation. > > Running configure by hand after that works fine. Why would I need to use > m4_pattern_allow for an autoconf built-in test? Never mind. Surprisingly, AC_USE_SYSTEM_MACROS is not the same as AC_USE_SYSTEM_EXTENSIONS. Duh. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkpSfQAACgkQX1gOwKyEAw8z5QCfXwZz2PozjdwAoT8Qc9aKTuup jIMAoIMxP/J5ar4nJOX62tZWdZGOUBuS =AX++ -----END PGP SIGNATURE----- |
From: Ian R. <id...@fr...> - 2009-07-06 17:20:05
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Pushed. Thanks. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkpSMhEACgkQX1gOwKyEAw+ISgCfcedpD30PiXSDtSZqLNCCGArS phwAoIt68Y94F/2i3du3tU+7+YX+5Jhc =P5YO -----END PGP SIGNATURE----- |
From: Julien C. <jcr...@de...> - 2009-07-04 19:47:05
|
On Sat, Jul 4, 2009 at 02:18:50 +0300, Pauli Nieminen wrote: > --- > libdrm/xf86drm.h | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/libdrm/xf86drm.h b/libdrm/xf86drm.h > index c1d173c..a7f9763 100644 > --- a/libdrm/xf86drm.h > +++ b/libdrm/xf86drm.h > @@ -37,6 +37,7 @@ > #include <stdarg.h> > #include <sys/types.h> > #include <stdint.h> > +#include <strings.h> > #include <drm.h> > > /* Defaults, if nothing set in xf86config */ commit message says string.h, patch has strings.h. Which is it? Cheers, Julien |
From: Pauli N. <su...@gm...> - 2009-07-05 07:08:57
|
strings.h (strcasecmp is posix function) On Sat, Jul 4, 2009 at 10:32 PM, Julien Cristau<jcr...@de...> wrote: > On Sat, Jul 4, 2009 at 02:18:50 +0300, Pauli Nieminen wrote: > >> --- >> libdrm/xf86drm.h | 1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> diff --git a/libdrm/xf86drm.h b/libdrm/xf86drm.h >> index c1d173c..a7f9763 100644 >> --- a/libdrm/xf86drm.h >> +++ b/libdrm/xf86drm.h >> @@ -37,6 +37,7 @@ >> #include <stdarg.h> >> #include <sys/types.h> >> #include <stdint.h> >> +#include <strings.h> >> #include <drm.h> >> >> /* Defaults, if nothing set in xf86config */ > > commit message says string.h, patch has strings.h. Which is it? > > Cheers, > Julien > |
From: Pauli N. <su...@gm...> - 2009-07-05 15:44:05
|
strcasecmp is declared in strings.h --- libdrm/xf86drm.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/libdrm/xf86drm.h b/libdrm/xf86drm.h index c1d173c..a7f9763 100644 --- a/libdrm/xf86drm.h +++ b/libdrm/xf86drm.h @@ -37,6 +37,7 @@ #include <stdarg.h> #include <sys/types.h> #include <stdint.h> +#include <strings.h> #include <drm.h> /* Defaults, if nothing set in xf86config */ -- 1.6.3.1 |
From: Ian R. <id...@fr...> - 2009-07-06 16:41:28
|
On Sun, Jul 05, 2009 at 06:43:40PM +0300, Pauli Nieminen wrote: > strcasecmp is declared in strings.h I pushed a modified version of this. strcasecmp is only used in xf86drm.c, so strings.h is included directly in that file. Thanks. |
From: Ian R. <id...@fr...> - 2009-07-06 16:41:47
|
Pushed. Thanks. |
From: Pauli N. <su...@gm...> - 2009-07-03 23:19:22
|
--- configure.ac | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index be2fbbf..763464b 100644 --- a/configure.ac +++ b/configure.ac @@ -52,6 +52,11 @@ AC_ARG_ENABLE(radeon-experimental-api, [Enable support for radeon's KMS API (default: disabled)]), [RADEON=$enableval], [RADEON=no]) +AC_ARG_ENABLE(strict-compilation, + AS_HELP_STRING([--enable-strict-compilation], + [Enable all warnings from compiler and make them errors (default: disabled)]), + [STRICT_COMPILE=$enableval], [STRICT_COMPILE=no]) + dnl =========================================================================== dnl check compiler flags AC_DEFUN([LIBDRM_CC_TRY_FLAG], [ @@ -93,7 +98,7 @@ MAYBE_WARN="-Wall -Wextra \ -Wstrict-aliasing=2 -Winit-self -Wunsafe-loop-optimizations \ -Wdeclaration-after-statement -Wold-style-definition \ -Wno-missing-field-initializers -Wno-unused-parameter \ --Wno-attributes -Wno-long-long -Winline" +-Wno-attributes -Wno-long-long -Winline -pedantic" # invalidate cached value if MAYBE_WARN has changed if test "x$libdrm_cv_warn_maybe" != "x$MAYBE_WARN"; then @@ -124,6 +129,10 @@ AC_CACHE_CHECK([for supported warning flags], libdrm_cv_warn_cflags, [ AC_MSG_CHECKING([which warning flags were supported])]) WARN_CFLAGS="$libdrm_cv_warn_cflags" +if test "x$STRICT_COMPILE" = xyes; then + CFLAGS="$CFLAGS $WARN_CLAGS -Werror" +fi + if test "x$UDEV" = xyes; then AC_DEFINE(UDEV, 1, [Have UDEV support]) fi -- 1.6.3.1 |
From: Pauli N. <su...@gm...> - 2009-07-03 23:19:31
|
--- libdrm/xf86drm.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 46 insertions(+), 3 deletions(-) diff --git a/libdrm/xf86drm.c b/libdrm/xf86drm.c index 7b05386..bf80fd4 100644 --- a/libdrm/xf86drm.c +++ b/libdrm/xf86drm.c @@ -269,6 +269,49 @@ static int drmMatchBusID(const char *id1, const char *id2) return 0; } +static int chownCheckReturn(const char *path, uid_t owner, gid_t group) +{ + int rv; + do { + rv = chown(path, owner, group); + } while( rv != 0 && errno == EINTR); + if (rv == 0) + return 0; + char *errMsg = ""; + switch(errno) + { + case EACCES: + errMsg = "Read permission denied."; + break; + case ELOOP: + errMsg = "Recursive symbolic link."; + break; + case ENAMETOOLONG: + errMsg = "Too long path."; + break; + case ENOTDIR: + errMsg = "Path isn't in a directory."; + break; + case ENOENT: + errMsg = "No file found."; + break; + case EPERM: + errMsg = "No permission to change the permission."; + break; + case EROFS: + errMsg = "Read-only file system."; + break; + case EIO: + errMsg = "I/O error."; + break; + case EINVAL: + errMsg = "The owner or group id is invalid."; + break; + } + drmMsg("Failed to change ower or group for file %s! %d: %s\n",path, errno, errMsg); + return -1; +} + /** * Open the DRM device, creating it if necessary. * @@ -307,7 +350,7 @@ static int drmOpenDevice(long dev, int minor, int type) if (!isroot) return DRM_ERR_NOT_ROOT; mkdir(DRM_DIR_NAME, DRM_DEV_DIRMODE); - chown(DRM_DIR_NAME, 0, 0); /* root:root */ + chownCheckReturn(DRM_DIR_NAME, 0, 0); /* root:root */ chmod(DRM_DIR_NAME, DRM_DEV_DIRMODE); } @@ -320,7 +363,7 @@ static int drmOpenDevice(long dev, int minor, int type) } if (drm_server_info) { - chown(buf, user, group); + chownCheckReturn(buf, user, group); chmod(buf, devmode); } #else @@ -363,7 +406,7 @@ wait_for_udev: remove(buf); mknod(buf, S_IFCHR | devmode, dev); if (drm_server_info) { - chown(buf, user, group); + chownCheckReturn(buf, user, group); chmod(buf, devmode); } } -- 1.6.3.1 |
From: Pauli N. <su...@gm...> - 2009-07-03 23:37:24
|
This might also require -std=c99 to set c standard correctly. I did use that in my CFLAGS when I tried to compile with -Werror. |
From: Pekka P. <pq...@ik...> - 2009-07-04 08:14:58
|
On Sat, 4 Jul 2009 02:18:53 +0300 Pauli Nieminen <su...@gm...> wrote: > --- > libdrm/xf86drm.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++--- > 1 files changed, 46 insertions(+), 3 deletions(-) > > diff --git a/libdrm/xf86drm.c b/libdrm/xf86drm.c > index 7b05386..bf80fd4 100644 > --- a/libdrm/xf86drm.c > +++ b/libdrm/xf86drm.c > @@ -269,6 +269,49 @@ static int drmMatchBusID(const char *id1, const char *id2) > return 0; > } > > +static int chownCheckReturn(const char *path, uid_t owner, gid_t group) > +{ > + int rv; > + do { > + rv = chown(path, owner, group); > + } while( rv != 0 && errno == EINTR); > + if (rv == 0) > + return 0; > + char *errMsg = ""; > + switch(errno) > + { > + case EACCES: > + errMsg = "Read permission denied."; > + break; > + case ELOOP: > + errMsg = "Recursive symbolic link."; > + break; > + case ENAMETOOLONG: > + errMsg = "Too long path."; > + break; > + case ENOTDIR: > + errMsg = "Path isn't in a directory."; > + break; > + case ENOENT: > + errMsg = "No file found."; > + break; > + case EPERM: > + errMsg = "No permission to change the permission."; > + break; > + case EROFS: > + errMsg = "Read-only file system."; > + break; > + case EIO: > + errMsg = "I/O error."; > + break; > + case EINVAL: > + errMsg = "The owner or group id is invalid."; > + break; > + } > + drmMsg("Failed to change ower or group for file %s! %d: %s\n",path, errno, errMsg); > + return -1; > +} > + Is there a reason for not using strerror() here? > /** > * Open the DRM device, creating it if necessary. > * > @@ -307,7 +350,7 @@ static int drmOpenDevice(long dev, int minor, int type) > if (!isroot) > return DRM_ERR_NOT_ROOT; > mkdir(DRM_DIR_NAME, DRM_DEV_DIRMODE); > - chown(DRM_DIR_NAME, 0, 0); /* root:root */ > + chownCheckReturn(DRM_DIR_NAME, 0, 0); /* root:root */ > chmod(DRM_DIR_NAME, DRM_DEV_DIRMODE); > } > > @@ -320,7 +363,7 @@ static int drmOpenDevice(long dev, int minor, int type) > } > > if (drm_server_info) { > - chown(buf, user, group); > + chownCheckReturn(buf, user, group); > chmod(buf, devmode); > } > #else > @@ -363,7 +406,7 @@ wait_for_udev: > remove(buf); > mknod(buf, S_IFCHR | devmode, dev); > if (drm_server_info) { > - chown(buf, user, group); > + chownCheckReturn(buf, user, group); > chmod(buf, devmode); > } > } > -- > 1.6.3.1 -- Pekka Paalanen http://www.iki.fi/pq/ |
From: Pauli N. <su...@gm...> - 2009-07-05 15:51:32
Attachments:
0005-libdrm-Make-chown-check-for-return-value.patch
|
Attached improved patch. On Sat, Jul 4, 2009 at 11:14 AM, Pekka Paalanen<pq...@ik...> wrote: > On Sat, 4 Jul 2009 02:18:53 +0300 > Pauli Nieminen <su...@gm...> wrote: > >> --- >> libdrm/xf86drm.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++--- >> 1 files changed, 46 insertions(+), 3 deletions(-) >> >> diff --git a/libdrm/xf86drm.c b/libdrm/xf86drm.c >> index 7b05386..bf80fd4 100644 >> --- a/libdrm/xf86drm.c >> +++ b/libdrm/xf86drm.c >> @@ -269,6 +269,49 @@ static int drmMatchBusID(const char *id1, const char *id2) >> return 0; >> } >> >> +static int chownCheckReturn(const char *path, uid_t owner, gid_t group) >> +{ >> + int rv; >> + do { >> + rv = chown(path, owner, group); >> + } while( rv != 0 && errno == EINTR); >> + if (rv == 0) >> + return 0; >> + char *errMsg = ""; >> + switch(errno) >> + { >> + case EACCES: >> + errMsg = "Read permission denied."; >> + break; >> + case ELOOP: >> + errMsg = "Recursive symbolic link."; >> + break; >> + case ENAMETOOLONG: >> + errMsg = "Too long path."; >> + break; >> + case ENOTDIR: >> + errMsg = "Path isn't in a directory."; >> + break; >> + case ENOENT: >> + errMsg = "No file found."; >> + break; >> + case EPERM: >> + errMsg = "No permission to change the permission."; >> + break; >> + case EROFS: >> + errMsg = "Read-only file system."; >> + break; >> + case EIO: >> + errMsg = "I/O error."; >> + break; >> + case EINVAL: >> + errMsg = "The owner or group id is invalid."; >> + break; >> + } >> + drmMsg("Failed to change ower or group for file %s! %d: %s\n",path, errno, errMsg); >> + return -1; >> +} >> + > > Is there a reason for not using strerror() here? > No good one. Only reason was that I have never used it before. > >> /** >> * Open the DRM device, creating it if necessary. >> * >> @@ -307,7 +350,7 @@ static int drmOpenDevice(long dev, int minor, int type) >> if (!isroot) >> return DRM_ERR_NOT_ROOT; >> mkdir(DRM_DIR_NAME, DRM_DEV_DIRMODE); >> - chown(DRM_DIR_NAME, 0, 0); /* root:root */ >> + chownCheckReturn(DRM_DIR_NAME, 0, 0); /* root:root */ >> chmod(DRM_DIR_NAME, DRM_DEV_DIRMODE); >> } >> >> @@ -320,7 +363,7 @@ static int drmOpenDevice(long dev, int minor, int type) >> } >> >> if (drm_server_info) { >> - chown(buf, user, group); >> + chownCheckReturn(buf, user, group); >> chmod(buf, devmode); >> } >> #else >> @@ -363,7 +406,7 @@ wait_for_udev: >> remove(buf); >> mknod(buf, S_IFCHR | devmode, dev); >> if (drm_server_info) { >> - chown(buf, user, group); >> + chownCheckReturn(buf, user, group); >> chmod(buf, devmode); >> } >> } >> -- >> 1.6.3.1 > > > -- > Pekka Paalanen > http://www.iki.fi/pq/ > |
From: Pauli N. <su...@gm...> - 2009-07-05 15:53:15
|
On Sat, Jul 4, 2009 at 2:37 AM, Pauli Nieminen<su...@gm...> wrote: > This might also require -std=c99 to set c standard correctly. I did > use that in my CFLAGS when I tried to compile with -Werror. > New version of patch attached to add the -std=c99 if using strict compilation |
From: Pauli N. <su...@gm...> - 2009-07-05 15:56:51
|
On Sun, Jul 5, 2009 at 6:53 PM, Pauli Nieminen<su...@gm...> wrote: > On Sat, Jul 4, 2009 at 2:37 AM, Pauli Nieminen<su...@gm...> wrote: >> This might also require -std=c99 to set c standard correctly. I did >> use that in my CFLAGS when I tried to compile with -Werror. >> > > New version of patch attached to add the -std=c99 if using strict compilation > I should learn to type and test everything well before submitting. |
From: Ian R. <id...@fr...> - 2009-07-06 21:36:34
|
On Sun, Jul 05, 2009 at 06:56:41PM +0300, Pauli Nieminen wrote: > @@ -124,6 +129,10 @@ AC_CACHE_CHECK([for supported warning flags], libdrm_cv_warn_cflags, [ > AC_MSG_CHECKING([which warning flags were supported])]) > WARN_CFLAGS="$libdrm_cv_warn_cflags" > > +if test "x$STRICT_COMPILE" = xyes; then > + CFLAGS="$CFLAGS $WARN_CFLAGS -std=c99 -Werror" I think instead you should use AC_PROG_CC_C99. I believe that this should always be used or -pendantic generates a number of spurious warnings. Does anyone object to force-enabling C99? There is at least one place in inteldrm where a C99 extension (variadic macros) is actually used. |