Secure file system utility functions

For Khronos, I've had to develop these utility functions to help me deal with storing the .wav files. However, they could also be used in a variety of applications.

A description of what the three functions do are given in the header documentation comments.

util.h:

#ifndef UTIL_H
#define UTIL_H

/**
 * @return the temporary file directory for your system
 */
const char* getTmpDir(void);

/**
 * Generates a unique temporary filename given the fileroot, creates and opens the file
 * @return file descriptor to the open file
 */
int createSafeFileDescriptor(const char* fileRoot);

/**
 * Fetches the file path given the file descriptor
 * @return file path
 */
const char* getPathFromDescriptor(int fd);

#endif // UTIL_H

util.c:

```
#include
#include
#include

#ifndef _WIN32
#include
#include
#endif

#include "util.h"

const char* getTmpDir(void)
{
char *tmpdir = NULL;
if ((tmpdir = getenv("TEMP"))) return tmpdir;
else if ((tmpdir = getenv("TMP"))) return tmpdir;
else if ((tmpdir = getenv("TMPDIR"))) return tmpdir;
else return "/tmp/";
}

int createSafeFileDescriptor(const char* fileRoot)
{
// Creates temporary file safely
char flacFile[FILENAME_MAX] = "";
snprintf(flacFile, FILENAME_MAX, "%sXXXXXX.wav", fileRoot);

// the 5 is for the length of the suffix ".wav"
return mkstemps(flacFile, 4);
}

const char* getPathFromDescriptor(int fd)
{
char *filename = malloc(FILENAME_MAX);
#ifdef _WIN32
intptr_t file = _get_osfhandle(fd);
intptr_t fileMap = CreateFileMapping(file, NULL, PAGE_READONLY, 0, 1, NULL);

if (hFileMap)
{
void* pMem = MapViewOfFile(hFileMap, FILE_MAP_READ, 0, 0, 1);
if (pMem)
{
if (GetMappedFileName(GetCurrentProcess(), pMem, filename, FILENAME_MAX)) return filename;
}
}
return NULL;
#endif

#ifdef __APPLE__
if (fcntl(fd, F_GETPATH, filename) != -1) return filename;
return NULL;
#endif

#ifdef __unix__
char proclnk[P

Good set of code.

Observations

-
#ifdef structure lacks uniqueness and detection if none are true. Suggest #elif

#ifdef _WIN32
#elif __APPLE__
#elif __unix__
#else
#error Error Message
#endif

-
Rather than a differing #if structure in the .c file, suggest a matching one as in #1

//#ifndef _WIN32
//#endif
#ifdef _WIN32
#elif __APPLE__
...

-
Memory leak. Should free(filename) before returning NULL.

if ((r = readlink(proclnk, filename, FILENAME_MAX)) < 0) return NULL;

-
Name-space. Prefer some naming convention that ties these 3 functions to
their .h file. Maybe wavutil?

wavutil.h
wavutil_getTmpDir();
wavutil_createSafeFileDescriptor()
wavutil_getPathFromDescriptor();

-
Since function allocates data, make that clear in the function declaration. Also document candidate error returns

* ALLOCATES the file path given the file descriptor
* Returns NULL on ...
...
const char* getPathFromDescriptor(int fd);

-
Suggest error return of NULL instead. Hoping "/tmp/" exists after failing 3 environment variables it a questionable fix. At least "./" is sure to exist. Also the environment variable might not end with /. May need to check that.

// else return "/tmp/";
else return NULL;
// or 
else return "./";

-
snprintf() is good to prevent overruns, yet if it fails, do not proceed with a runt string.

int len = snprintf(proclnk, sizeof(proclnk), "/proc/self/fd/%d", fd);
if (len = sizeof(proclnk)) Handle_Error();
if ((r = readlink(proclnk, filename, FILENAME_MAX)) < 0) return NULL;

Minor

-
Comment inconsistent with code 5 vs. 4? Could avoid magic number 4.

// // the 5 is for the length of the suffix ".wav"
   // return mkstemps(flacFile, 4);

   char ext[] = ".wav";
   snprintf(flacFile, FILENAME_MAX, "%sXXXXXX%s", fileRoot, ext);
   return mkstemps(flacFile, sizeof ext - 1);

-
Prefer right-sizing allocations that start off large.

char *filename = malloc(FILENAME_MAX);
   ...
   char *p = realloc(filename, strlen(filename) + 1);
   return p ? p : filename;

-
Curious style change. Why sizeof() in 2nd line, yet FILENAME_MAX instead of sizeof(flacFile). Suggest the latter. Further, not a fan of xxx_MAX when xxx_SIZE is meant. MAX to me means the maximum number so characters - which is 1 less than size.

char proclnk[PATH_MAX] = {0};
snprintf(proclnk, sizeof(proclnk), "/proc/self/fd/%d", fd);

char flacFile[FILENAME_MAX] = "";
snprintf(flacFile, FILENAME_MAX, "%sXXXXXX.wav", fileRoot);

StackExchange Code Review Q#128120, answer score: 6