| Message ID | 20180326102648.1754-1-eric.engestrom@imgtec.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 26 Mar 2018, Eric Engestrom <eric.engestrom@imgtec.com> wrote: > Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com> > --- > xf86drm.c | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/xf86drm.c b/xf86drm.c > index 3a9d0ed2cc9b196ae7d1..b6e5d8cc1bb50ffe75a2 100644 > --- a/xf86drm.c > +++ b/xf86drm.c > @@ -2823,7 +2823,7 @@ static char *drmGetMinorNameForFD(int fd, int type) > struct stat sbuf; > const char *name = drmGetMinorName(type); > int len; > - char dev_name[64], buf[64]; > + char *dev_name, buf[64]; > int maj, min; > > if (!name) > @@ -2848,20 +2848,22 @@ static char *drmGetMinorNameForFD(int fd, int type) > > while ((ent = readdir(sysdir))) { > if (strncmp(ent->d_name, name, len) == 0) { > - snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s", > - ent->d_name); > + if (asprintf(&dev_name, DRM_DIR_NAME "/%s", Just noting in passing that asprintf is a GNU extension, is that okay? BR, Jani. > + ent->d_name) < 0) { > + dev_name = NULL; > + } > > closedir(sysdir); > - return strdup(dev_name); > + return dev_name; > } > } > return NULL; > #else > struct stat sbuf; > - char buf[PATH_MAX + 1]; > + char *buf; > const char *dev_name; > unsigned int maj, min; > - int n, base; > + int base; > > if (fstat(fd, &sbuf)) > return NULL; > @@ -2890,11 +2892,10 @@ static char *drmGetMinorNameForFD(int fd, int type) > if (base < 0) > return NULL; > > - n = snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, min - base); > - if (n == -1 || n >= sizeof(buf)) > + if (asprintf(&buf, dev_name, DRM_DIR_NAME, min - base) < 0) > return NULL; > > - return strdup(buf); > + return buf; > #endif > } > > @@ -4119,10 +4120,10 @@ char *drmGetDeviceNameFromFd2(int fd) > return strdup(path); > #else > struct stat sbuf; > - char node[PATH_MAX + 1]; > + char *node; > const char *dev_name; > int node_type; > - int maj, min, n, base; > + int maj, min, base; > > if (fstat(fd, &sbuf)) > return NULL; > @@ -4155,11 +4156,10 @@ char *drmGetDeviceNameFromFd2(int fd) > if (base < 0) > return NULL; > > - n = snprintf(node, PATH_MAX, dev_name, DRM_DIR_NAME, min - base); > - if (n == -1 || n >= PATH_MAX) > + if (asprintf(&node, dev_name, DRM_DIR_NAME, min - base) < 0) > return NULL; > > - return strdup(node); > + return node; > #endif > }
On 26 March 2018 at 14:57, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Mon, 26 Mar 2018, Eric Engestrom <eric.engestrom@imgtec.com> wrote: >> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com> >> --- >> xf86drm.c | 28 ++++++++++++++-------------- >> 1 file changed, 14 insertions(+), 14 deletions(-) >> >> diff --git a/xf86drm.c b/xf86drm.c >> index 3a9d0ed2cc9b196ae7d1..b6e5d8cc1bb50ffe75a2 100644 >> --- a/xf86drm.c >> +++ b/xf86drm.c >> @@ -2823,7 +2823,7 @@ static char *drmGetMinorNameForFD(int fd, int type) >> struct stat sbuf; >> const char *name = drmGetMinorName(type); >> int len; >> - char dev_name[64], buf[64]; >> + char *dev_name, buf[64]; >> int maj, min; >> >> if (!name) >> @@ -2848,20 +2848,22 @@ static char *drmGetMinorNameForFD(int fd, int type) >> >> while ((ent = readdir(sysdir))) { >> if (strncmp(ent->d_name, name, len) == 0) { >> - snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s", >> - ent->d_name); >> + if (asprintf(&dev_name, DRM_DIR_NAME "/%s", > > Just noting in passing that asprintf is a GNU extension, is that okay? > Was going to mention the same thing. Also POSIX please add it to the next version ;-) It doesn't seem to make the code shorter, so I'd go with let's drop this? -Emil
On Monday, 2018-03-26 15:00:01 +0100, Emil Velikov wrote: > On 26 March 2018 at 14:57, Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Mon, 26 Mar 2018, Eric Engestrom <eric.engestrom@imgtec.com> wrote: > >> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com> > >> --- > >> xf86drm.c | 28 ++++++++++++++-------------- > >> 1 file changed, 14 insertions(+), 14 deletions(-) > >> > >> diff --git a/xf86drm.c b/xf86drm.c > >> index 3a9d0ed2cc9b196ae7d1..b6e5d8cc1bb50ffe75a2 100644 > >> --- a/xf86drm.c > >> +++ b/xf86drm.c > >> @@ -2823,7 +2823,7 @@ static char *drmGetMinorNameForFD(int fd, int type) > >> struct stat sbuf; > >> const char *name = drmGetMinorName(type); > >> int len; > >> - char dev_name[64], buf[64]; > >> + char *dev_name, buf[64]; > >> int maj, min; > >> > >> if (!name) > >> @@ -2848,20 +2848,22 @@ static char *drmGetMinorNameForFD(int fd, int type) > >> > >> while ((ent = readdir(sysdir))) { > >> if (strncmp(ent->d_name, name, len) == 0) { > >> - snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s", > >> - ent->d_name); > >> + if (asprintf(&dev_name, DRM_DIR_NAME "/%s", > > > > Just noting in passing that asprintf is a GNU extension, is that okay? > > > Was going to mention the same thing. Also POSIX please add it to the > next version ;-) > It doesn't seem to make the code shorter, so I'd go with let's drop this? Those were just some local changes I had done at some random point when coming across stuff that I thought could be better. I just sent them now since my local changes will be lost by me leaving my current job, and I didn't care enough to push them on a branch somewhere (: Either these were thought to be good by someone else, or, as it is, they're not and I'm dropping all three :)
diff --git a/xf86drm.c b/xf86drm.c index 3a9d0ed2cc9b196ae7d1..b6e5d8cc1bb50ffe75a2 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -2823,7 +2823,7 @@ static char *drmGetMinorNameForFD(int fd, int type) struct stat sbuf; const char *name = drmGetMinorName(type); int len; - char dev_name[64], buf[64]; + char *dev_name, buf[64]; int maj, min; if (!name) @@ -2848,20 +2848,22 @@ static char *drmGetMinorNameForFD(int fd, int type) while ((ent = readdir(sysdir))) { if (strncmp(ent->d_name, name, len) == 0) { - snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s", - ent->d_name); + if (asprintf(&dev_name, DRM_DIR_NAME "/%s", + ent->d_name) < 0) { + dev_name = NULL; + } closedir(sysdir); - return strdup(dev_name); + return dev_name; } } return NULL; #else struct stat sbuf; - char buf[PATH_MAX + 1]; + char *buf; const char *dev_name; unsigned int maj, min; - int n, base; + int base; if (fstat(fd, &sbuf)) return NULL; @@ -2890,11 +2892,10 @@ static char *drmGetMinorNameForFD(int fd, int type) if (base < 0) return NULL; - n = snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, min - base); - if (n == -1 || n >= sizeof(buf)) + if (asprintf(&buf, dev_name, DRM_DIR_NAME, min - base) < 0) return NULL; - return strdup(buf); + return buf; #endif } @@ -4119,10 +4120,10 @@ char *drmGetDeviceNameFromFd2(int fd) return strdup(path); #else struct stat sbuf; - char node[PATH_MAX + 1]; + char *node; const char *dev_name; int node_type; - int maj, min, n, base; + int maj, min, base; if (fstat(fd, &sbuf)) return NULL; @@ -4155,11 +4156,10 @@ char *drmGetDeviceNameFromFd2(int fd) if (base < 0) return NULL; - n = snprintf(node, PATH_MAX, dev_name, DRM_DIR_NAME, min - base); - if (n == -1 || n >= PATH_MAX) + if (asprintf(&node, dev_name, DRM_DIR_NAME, min - base) < 0) return NULL; - return strdup(node); + return node; #endif }
Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com> --- xf86drm.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)