Make changes according to recommendations by Copilot

- correction spelling of suppress_error
- improved error handling in remove() based on functionality in get_fileattr() and somewhat changed error handling in get_fileattr() itself
- call explicitly LLFile::fopen() to make sure we use the correct file path conversion under Windows

Removing Flawfinder comments since Flawfinder isn't used in the viewer anymore
Adding an option to support symlink detection in getattr()
Adding comments to function implementation to indicate that they are really static functions of the LLFile class
master
Frederick Martian 2025-10-18 19:13:19 +02:00 committed by Andrey Kleshchev
parent 2adf1bbdcb
commit f58a7c7673
2 changed files with 122 additions and 76 deletions

View File

@ -48,15 +48,18 @@ static std::string empty;
// variants of strerror() to report errors.
#if LL_WINDOWS
// For the situations where we directly call into Windows API functions we need to translate
// the Windows error codes into errno values
namespace
{
struct errentry
{
unsigned long oserr; // OS return value
unsigned long oserr; // Windows OS error value
int errcode; // System V error code
};
}
// translation table between Windows OS error value and System V errno code
static errentry const errtable[]
{
{ ERROR_INVALID_FUNCTION, EINVAL }, // 1
@ -106,20 +109,17 @@ static errentry const errtable[]
{ ERROR_NOT_ENOUGH_QUOTA, ENOMEM } // 1816
};
// Number of elements in the error translation table
#define ERRTABLECOUNT (sizeof(errtable) / sizeof(errtable[0]))
static int set_errno_from_oserror(unsigned long oserr)
{
if (!oserr)
return 0;
// Check the table for the OS error code
for (unsigned i{0}; i < ERRTABLECOUNT; ++i)
// Check the table for the Windows OS error code
for (const struct errentry &entry : errtable)
{
if (oserr == errtable[i].oserr)
if (oserr == entry.oserr)
{
_set_errno(errtable[i].errcode);
_set_errno(entry.errcode);
return -1;
}
}
@ -156,18 +156,18 @@ static std::wstring utf8path_to_wstring(const std::string& utf8path)
return ll_convert<std::wstring>(utf8path);
}
static unsigned short get_fileattr(const std::wstring& utf16path)
static unsigned short get_fileattr(const std::wstring& utf16path, bool dontFollowSymLink = false)
{
unsigned short st_mode = 0;
HANDLE file_handle = CreateFileW(utf16path.c_str(), FILE_READ_ATTRIBUTES,
FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, nullptr);
if (file_handle == INVALID_HANDLE_VALUE)
unsigned long flags = FILE_FLAG_BACKUP_SEMANTICS;
if (dontFollowSymLink)
{
set_errno_from_oserror(GetLastError());
return 0;
flags |= FILE_FLAG_OPEN_REPARSE_POINT;
}
HANDLE file_handle = CreateFileW(utf16path.c_str(), FILE_READ_ATTRIBUTES,
FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
nullptr, OPEN_EXISTING, flags, nullptr);
if (file_handle != INVALID_HANDLE_VALUE)
{
FILE_ATTRIBUTE_TAG_INFO attribute_info;
if (GetFileInformationByHandleEx(file_handle, FileAttributeTagInfo, &attribute_info, sizeof(attribute_info)))
{
@ -175,22 +175,28 @@ static unsigned short get_fileattr(const std::wstring& utf16path)
bool is_directory = (attribute_info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY) ||
(iswalpha(utf16path[0]) && utf16path[1] == ':' &&
(!utf16path[2] || (is_slash(utf16path[2]) && !utf16path[3])));
st_mode |= is_directory ? S_IFDIR : S_IFREG;
unsigned short st_mode = is_directory ? S_IFDIR :
(attribute_info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT ? S_IFLNK : S_IFREG);
st_mode |= (attribute_info.FileAttributes & FILE_ATTRIBUTE_READONLY) ? S_IREAD : S_IREAD | S_IWRITE;
// we do not try to guess executable flag
// propagate user bits to group/other fields:
st_mode |= (st_mode & 0700) >> 3;
st_mode |= (st_mode & 0700) >> 6;
}
else
{
// Retrieve last error before calling CloseHandle()
set_errno_from_oserror(GetLastError());
}
CloseHandle(file_handle);
return st_mode;
}
}
// Retrieve last error and set errno before calling CloseHandle()
set_errno_from_oserror(GetLastError());
if (file_handle != INVALID_HANDLE_VALUE)
{
CloseHandle(file_handle);
}
return 0;
}
#else
// On Posix we want to call strerror_r(), but alarmingly, there are two
@ -241,7 +247,7 @@ std::string strerr(int errn)
}
#endif // ! LL_WINDOWS
int warnif(const std::string& desc, const std::string& filename, int rc, int accept = 0)
static int warnif(const std::string& desc, const std::string& filename, int rc, int accept = 0)
{
if (rc < 0)
{
@ -333,7 +339,7 @@ int LLFile::mkdir(const std::string& dirname, int perms)
}
// static
int LLFile::rmdir(const std::string& dirname, int supress_error)
int LLFile::rmdir(const std::string& dirname, int suppress_error)
{
#if LL_WINDOWS
std::wstring utf16dirname = utf8path_to_wstring(dirname);
@ -341,21 +347,22 @@ int LLFile::rmdir(const std::string& dirname, int supress_error)
#else
int rc = ::rmdir(dirname.c_str());
#endif
return warnif("rmdir", dirname, rc, supress_error);
return warnif("rmdir", dirname, rc, suppress_error);
}
// static
LLFILE* LLFile::fopen(const std::string& filename, const char* mode) /* Flawfinder: ignore */
LLFILE* LLFile::fopen(const std::string& filename, const char* mode)
{
#if LL_WINDOWS
std::wstring utf16filename = utf8path_to_wstring(filename);
std::wstring utf16mode = ll_convert<std::wstring>(std::string(mode));
return _wfopen(utf16filename.c_str(), utf16mode.c_str());
#else
return ::fopen(filename.c_str(),mode); /* Flawfinder: ignore */
return ::fopen(filename.c_str(),mode);
#endif
}
// static
int LLFile::close(LLFILE * file)
{
int ret_value = 0;
@ -366,9 +373,10 @@ int LLFile::close(LLFILE * file)
return ret_value;
}
// static
std::string LLFile::getContents(const std::string& filename)
{
LLFILE* fp = fopen(filename, "rb"); /* Flawfinder: ignore */
LLFILE* fp = LLFile::fopen(filename, "rb");
if (fp)
{
fseek(fp, 0, SEEK_END);
@ -385,7 +393,8 @@ std::string LLFile::getContents(const std::string& filename)
return LLStringUtil::null;
}
int LLFile::remove(const std::string& filename, int supress_error)
// static
int LLFile::remove(const std::string& filename, int suppress_error)
{
#if LL_WINDOWS
// Posix remove() works on both files and directories although on Windows
@ -393,8 +402,7 @@ int LLFile::remove(const std::string& filename, int supress_error)
// as its siblings unlink() and _wunlink().
// If we really only want to support files we should instead use
// unlink() in the non-Windows part below too
int rc = 0;
;
int rc = -1;
std::wstring utf16filename = utf8path_to_wstring(filename);
unsigned short st_mode = get_fileattr(utf16filename);
if (S_ISDIR(st_mode))
@ -405,17 +413,25 @@ int LLFile::remove(const std::string& filename, int supress_error)
{
rc = _wunlink(utf16filename.c_str());
}
else if (st_mode)
{
// it is something else than a file or directory
// this should not really happen as long as we do not allow for symlink
// detection in the optional parameter to get_fileattr()
rc = set_errno_from_oserror(ERROR_INVALID_PARAMETER);
}
else
{
rc = set_errno_from_oserror(ERROR_INVALID_PARAMETER);
// get_filattr() failed and already set errno, preserve it for correct error reporting
}
#else
int rc = ::remove(filename.c_str());
#endif
return warnif("remove", filename, rc, supress_error);
return warnif("remove", filename, rc, suppress_error);
}
int LLFile::rename(const std::string& filename, const std::string& newname, int supress_error)
// static
int LLFile::rename(const std::string& filename, const std::string& newname, int suppress_error)
{
#if LL_WINDOWS
// Posix rename() will gladly overwrite a file at newname if it exists, the Windows
@ -431,25 +447,26 @@ int LLFile::rename(const std::string& filename, const std::string& newname, int
#else
int rc = ::rename(filename.c_str(),newname.c_str());
#endif
return warnif(STRINGIZE("rename to '" << newname << "' from"), filename, rc, supress_error);
return warnif(STRINGIZE("rename to '" << newname << "' from"), filename, rc, suppress_error);
}
// Make this a define rather than using magic numbers multiple times in the code
#define LLFILE_COPY_BUFFER_SIZE 16384
// static
bool LLFile::copy(const std::string& from, const std::string& to)
{
bool copied = false;
LLFILE* in = fopen(from, "rb"); /* Flawfinder: ignore */
LLFILE* in = LLFile::fopen(from, "rb");
if (in)
{
LLFILE* out = fopen(to, "wb"); /* Flawfinder: ignore */
LLFILE* out = LLFile::fopen(to, "wb");
if (out)
{
char buf[LLFILE_COPY_BUFFER_SIZE]; /* Flawfinder: ignore */
char buf[LLFILE_COPY_BUFFER_SIZE];
size_t readbytes;
bool write_ok = true;
while (write_ok && (readbytes = fread(buf, 1, LLFILE_COPY_BUFFER_SIZE, in))) /* Flawfinder: ignore */
while (write_ok && (readbytes = fread(buf, 1, LLFILE_COPY_BUFFER_SIZE, in)))
{
if (fwrite(buf, 1, readbytes, out) != readbytes)
{
@ -468,7 +485,8 @@ bool LLFile::copy(const std::string& from, const std::string& to)
return copied;
}
int LLFile::stat(const std::string& filename, llstat* filestatus, int supress_error)
// static
int LLFile::stat(const std::string& filename, llstat* filestatus, int suppress_error)
{
#if LL_WINDOWS
std::wstring utf16filename = utf8path_to_wstring(filename);
@ -476,24 +494,25 @@ int LLFile::stat(const std::string& filename, llstat* filestatus, int supress_er
#else
int rc = ::stat(filename.c_str(), filestatus);
#endif
return warnif("stat", filename, rc, supress_error);
return warnif("stat", filename, rc, suppress_error);
}
// _wstat() is a bit heavyweight on Windows, use a more lightweight API
// to just get the attributes
unsigned short LLFile::getattr(const std::string& filename, int suppress_error)
// static
unsigned short LLFile::getattr(const std::string& filename, bool dontFollowSymLink, int suppress_error)
{
#if LL_WINDOWS
// _wstat64() is a bit heavyweight on Windows, use a more lightweight API
// to just get the attributes
int rc = -1;
std::wstring utf16filename = utf8path_to_wstring(filename);
unsigned short st_mode = get_fileattr(utf16filename);
unsigned short st_mode = get_fileattr(utf16filename, dontFollowSymLink);
if (st_mode)
{
return st_mode;
}
#else
llstat filestatus;
int rc = ::stat(filename.c_str(), &filestatus);
int rc = dontFollowSymLink ? ::lstat(filename.c_str(), &filestatus) : ::stat(filename.c_str(), &filestatus);
if (rc == 0)
{
return filestatus.st_mode;
@ -503,18 +522,25 @@ unsigned short LLFile::getattr(const std::string& filename, int suppress_error)
return 0;
}
// static
bool LLFile::isdir(const std::string& filename)
{
// Don't spam the log if the subject pathname doesn't exist.
return S_ISDIR(getattr(filename, ENOENT));
return S_ISDIR(getattr(filename));
}
// static
bool LLFile::isfile(const std::string& filename)
{
// Don't spam the log if the subject pathname doesn't exist.
return S_ISREG(getattr(filename, ENOENT));
return S_ISREG(getattr(filename));
}
// static
bool LLFile::islink(const std::string& filename)
{
return S_ISLNK(getattr(filename, true));
}
// static
const char *LLFile::tmpdir()
{
static std::string utf8path;

View File

@ -41,7 +41,8 @@ typedef FILE LLFILE;
#include <sys/stat.h>
#if LL_WINDOWS
// windows version of stat function and stat data structure are called _statxx
// The Windows version of stat function and stat data structure are called _stat64
// We use _stat64 here to support 64-bit st_size and time_t values
typedef struct _stat64 llstat;
#else
typedef struct stat llstat;
@ -56,6 +57,16 @@ typedef struct stat llstat;
# define S_ISDIR(x) (((x) & S_IFMT) == S_IFDIR)
#endif
// Windows C runtime library does not define this and does not support symlink detection in the
// stat functions but we do in our getattr() function
#ifndef S_IFLNK
#define S_IFLNK 0xA000 /* symlink */
#endif
#ifndef S_ISLNK
#define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)
#endif
#include "llstring.h" // safe char* -> std::string conversion
/// LLFile is a class of static functions operating on paths
@ -96,39 +107,44 @@ public:
/// @returns 0 on success and -1 on failure.
//// remove a directory
static int rmdir(const std::string& filename, int supress_error = 0);
///< pass ENOENT in the optional 'supress_error' parameter
static int rmdir(const std::string& filename, int suppress_error = 0);
///< pass ENOENT in the optional 'suppress_error' parameter
/// if you don't want a warning in the log when the directory does not exist
/// @returns 0 on success and -1 on failure.
/// remove a file or directory
static int remove(const std::string& filename, int supress_error = 0);
///< pass ENOENT in the optional 'supress_error' parameter
static int remove(const std::string& filename, int suppress_error = 0);
///< pass ENOENT in the optional 'suppress_error' parameter
/// if you don't want a warning in the log when the directory does not exist
/// @returns 0 on success and -1 on failure.
/// rename a file
static int rename(const std::string& filename, const std::string& newname, int supress_error = 0);
static int rename(const std::string& filename, const std::string& newname, int suppress_error = 0);
///< it will silently overwrite newname if it exists without returning an error
/// Posix guarantees that if newname already exists, then there will be no moment
/// in which for other processes newname does not exist. There is no such guarantee
/// under Windows at this time. It may do it in the same way but the used Windows API
/// does not make such guarantees.
/// @returns 0 on success and -1 on failure.
// copy the contents of file from 'from' to 'to' filename
/// copy the contents of file from 'from' to 'to' filename
static bool copy(const std::string& from, const std::string& to);
///< @returns true on success and false on failure.
/// return the file stat structure for filename
static int stat(const std::string& filename, llstat* file_status, int supress_error = ENOENT);
static int stat(const std::string& filename, llstat* file_status, int suppress_error = ENOENT);
///< for compatibility with existing uses of LL_File::stat() we use ENOENT as default in the
/// optional 'supress_error' parameter to avoid spamming the log with warnings when the API
/// optional 'suppress_error' parameter to avoid spamming the log with warnings when the API
/// is used to detect if a file exists
/// @returns 0 on success and -1 on failure.
/// get the file or directory attributes for filename
static unsigned short getattr(const std::string& filename, int supress_error = 0);
static unsigned short getattr(const std::string& filename, bool dontFollowSymLink = false, int suppress_error = ENOENT);
///< a more lightweight function on Windows to stat, that just returns the file attribute flags
/// pass ENOENT in the optional 'supress_error' parameter if you don't want a warning
/// in the log when the file or directory does not exist
/// dontFollowSymLinks set to true returns the attributes of the symlink if it is one, rather than resolving it
/// we pass by default ENOENT in the optional 'suppress_error' parameter to not spam the log with
/// warnings when the file or directory does not exist
/// @returns 0 on failure and a st_mode value with either S_IFDIR or S_IFREG set otherwise
/// together with the three access bits which under Windows only the write bit is relevant.
@ -140,6 +156,10 @@ public:
static bool isfile(const std::string& filename);
///< @returns true if the path is for an existing file
/// check if filename is a symlink
static bool islink(const std::string& filename);
///< @returns true if the path is pointing at a symlink
/// return a path to the temporary directory on the system
static const char * tmpdir();
};