Hi! I killed the camel case and added documentation comment before function.
It is possible to use macros to get similar error checking to all others system calls that would be security problem in case of failure. Pauli On Mon, Jul 6, 2009 at 7:48 PM, Ian Romanick<[email protected]> wrote: > On Sun, Jul 05, 2009 at 06:51:26PM +0300, Pauli Nieminen wrote: >> Attached improved patch. >> >> From cdf9faaab782f2a84ee16c27a4e7b1f70d2e6ad5 Mon Sep 17 00:00:00 2001 >> From: Pauli Nieminen <[email protected]> >> Date: Sun, 5 Jul 2009 18:31:18 +0300 >> Subject: [PATCH 5/6] libdrm: Make chown check for return value. >> If call was interrupted by signal we have to make call again. >> >> --- >> libdrm/xf86drm.c | 19 ++++++++++++++++--- >> 1 files changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/libdrm/xf86drm.c b/libdrm/xf86drm.c >> index 7b05386..3da0380 100644 >> --- a/libdrm/xf86drm.c >> +++ b/libdrm/xf86drm.c >> @@ -269,6 +269,19 @@ static int drmMatchBusID(const char *id1, const char >> *id2) >> return 0; >> } >> >> +static int chownCheckReturn(const char *path, uid_t owner, gid_t group) > > Camel case must die. :) chown_check_return instead, please. > >> +{ >> + int rv; >> + do { >> + rv = chown(path, owner, group); >> + } while( rv != 0 && errno == EINTR); >> + if (rv == 0) >> + return 0; >> + char *errMsg = strerror(errno); >> + drmMsg("Failed to change ower or group for file %s! %d: %s\n",path, >> errno, errMsg); > > Please wrap long lines. > >> + return -1; >> +} >> + >> /** >> * Open the DRM device, creating it if necessary. >> * >> @@ -307,7 +320,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 +333,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 +376,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 > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.9 (GNU/Linux) > > iEYEARECAAYFAkpSKu0ACgkQX1gOwKyEAw/5BACfRaJJTFYzeDl0b7RiMcrCUb71 > wWEAn3yIXlIRdxNs5HFBfqKjfyZ2xPzj > =GOSC > -----END PGP SIGNATURE----- > >
From e5727071becbdd9bdde035cf7b5235c1c485e600 Mon Sep 17 00:00:00 2001 From: Pauli Nieminen <[email protected]> Date: Mon, 6 Jul 2009 23:37:20 +0300 Subject: libdrm: Make chown check for return value. If call was interrupted by signal we have to make call again. --- libdrm/xf86drm.c | 36 +++++++++++++++++++++++++++++++++--- 1 files changed, 33 insertions(+), 3 deletions(-) diff --git a/libdrm/xf86drm.c b/libdrm/xf86drm.c index 7b05386..5d12c20 100644 --- a/libdrm/xf86drm.c +++ b/libdrm/xf86drm.c @@ -270,6 +270,36 @@ static int drmMatchBusID(const char *id1, const char *id2) } /** + * Handles error checking for chown call. + * + * \param path to file. + * \param id of the new owner. + * \param id of the new group. + * + * \return zero if success or -1 if failure. + * + * \internal + * Checks for failure. If failure was caused by signal call chown again. + * If any other failure happened then it will output error mesage using + * drmMsg() call. + */ +static int chown_check_return(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 = strerror(errno); + 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. * * \param dev major and minor numbers of the device. @@ -307,7 +337,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 */ + chown_check_return(DRM_DIR_NAME, 0, 0); /* root:root */ chmod(DRM_DIR_NAME, DRM_DEV_DIRMODE); } @@ -320,7 +350,7 @@ static int drmOpenDevice(long dev, int minor, int type) } if (drm_server_info) { - chown(buf, user, group); + chown_check_return(buf, user, group); chmod(buf, devmode); } #else @@ -363,7 +393,7 @@ wait_for_udev: remove(buf); mknod(buf, S_IFCHR | devmode, dev); if (drm_server_info) { - chown(buf, user, group); + chown_check_return(buf, user, group); chmod(buf, devmode); } } -- 1.6.3.1
------------------------------------------------------------------------------
-- _______________________________________________ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
