This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On 06/14/2018 08:01 AM, Adhemerval Zanella wrote:
+ char fdfilename[30];
The magic number 30 should be turned into a named constant defined in fd_to_filename.h, to help prevent future mistakes. Once that is done, you can change the signature of fd_to_filename to not pass the size, and to require the caller to pass an array of at least size 30, so that fd_to_filename need not check for buffer overflow (see below for more on this).
+ const char *gfilename; + if (filename == NULL && fd >= 0) + gfilename = fd_to_filename (fd, fdfilename, sizeof fdfilename) + ? fdfilename : NULL; + else + gfilename = filename;
Cleaner would be: const char *gfilename = filename != NULL ? filename : fd_to_filename (fd, fdfilename);That is, let fd_to_filename worry about what to do with negative fd, and have it return fdfilename or NULL, and don't pass the size (which should be that magic number regardless).
-static inline const char * -fd_to_filename (int fd) +static inline bool +fd_to_filename (int fd, char *buf, size_t len) { - char *ret = malloc (30); + __snprintf (buf, len, "/proc/self/fd/%d", fd);- if (ret != NULL)- { - struct stat64 st; - - *_fitoa_word (fd, __stpcpy (ret, "/proc/self/fd/"), 10, 0) = '\0'; - - /* We must make sure the file exists. */ - if (__lxstat64 (_STAT_VER, ret, &st) < 0) - { - /* /proc is not mounted or something else happened. Don't - return the file name. */ - free (ret); - ret = NULL; - } - } - return ret; + /* We must make sure the file exists. */ + if (__lxstat64 (_STAT_VER, buf, & (struct stat64) {}) < 0) + /* /proc is not mounted or something else happened. */ + return false; + return true; }
The __snprintf would be quite wrong if the string did not fit. Again, I suggest simply requiring the buffer to be long enough and not checking its length, and sticking with stpcpy + _fitoa_word which should be more efficient than __snprintf anyway (or if you prefer simplicity to speed, just use sprintf).
The '& (struct stat64) {}' construct looks pretty but is less efficient as it makes the compiler zero out the structure unnecessarily, so the code should keep doing that struct the old-fashioned way.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |