Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make project compile with clang 11.0.1-2 in Mac #3795

Merged
merged 3 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/include/OpenImageIO/string_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,6 @@ class basic_string_view {
using string_view = basic_string_view<char>;
using wstring_view = basic_string_view<wchar_t>;



// DEPRECATED name equivalence
OIIO_DEPRECATED("Use string_view (2.3)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please omit this change. We don't alter formatting or whitespace in code unrelated to the subject of a PR.

typedef string_view string_ref;
Expand Down
6 changes: 4 additions & 2 deletions src/include/OpenImageIO/strutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -670,13 +670,15 @@ template<> inline double from_string<double> (string_view s) {

template<> inline int64_t from_string<int64_t>(string_view s) {
// For conversion of string_view to unsigned int, fall back on strtoll.
auto r = strtoll(std::string(s).c_str(), nullptr, 10);
std::string s2 = std::string(s);
auto r = strtoll(s2.c_str(), nullptr, 10);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is fine, I guess, but I don't understand why it is necessary. What error message did you get with the old code?

return static_cast<int64_t>(r);
}

template<> inline uint64_t from_string<uint64_t>(string_view s) {
// For conversion of string_view to unsigned int, fall back on strtoull.
auto r = strtoull(std::string(s).c_str(), nullptr, 10);
std::string s2 = std::string(s);
auto r = strtoull(s2.c_str(), nullptr, 10);
return static_cast<uint64_t>(r);
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/libOpenImageIO/imagebuf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ ImageBufImpl::error(string_view message) const
// a single newline.
if (m_err.size() && m_err.back() != '\n')
m_err += '\n';
m_err += message;
m_err += std::string(message);
lgritz marked this conversation as resolved.
Show resolved Hide resolved
}


Expand Down
2 changes: 1 addition & 1 deletion src/libOpenImageIO/imageinput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@ ImageInput::append_error(string_view message) const
if (errptr->size() < 1024 * 1024 * 16) {
if (errptr->size() && errptr->back() != '\n')
*errptr += '\n';
*errptr += message;
*errptr += std::string(message);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/libOpenImageIO/imageio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,12 +233,12 @@ pvt::append_error(string_view message)
// a single newline.
if (error_msg.size() && error_msg.back() != '\n')
error_msg += '\n';
error_msg += message;
error_msg += std::string(message);

// Remove a single trailing newline
if (message.size() && message.back() == '\n')
message.remove_suffix(1);
error_msg = message;
error_msg = std::string(message);
}


Expand Down
2 changes: 1 addition & 1 deletion src/libOpenImageIO/imageoutput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ ImageOutput::append_error(string_view message) const
&& "Accumulated error messages > 16MB. Try checking return codes!");
if (errptr->size() && errptr->back() != '\n')
*errptr += '\n';
*errptr += message;
*errptr += std::string(message);
}


Expand Down
2 changes: 1 addition & 1 deletion src/libtexture/imagecache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3942,7 +3942,7 @@ ImageCacheImpl::append_error(string_view message) const
&& "Accumulated error messages > 16MB. Try checking return codes!");
if (errptr->size() && errptr->back() != '\n')
*errptr += '\n';
*errptr += message;
*errptr += std::string(message);
}


Expand Down
2 changes: 1 addition & 1 deletion src/libtexture/texturesys.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ TextureSystemImpl::append_error(string_view message) const
&& "Accumulated error messages > 16MB. Try checking return codes!");
if (errptr->size() && errptr->back() != '\n')
*errptr += '\n';
*errptr += message;
*errptr += std::string(message);
}


Expand Down
12 changes: 11 additions & 1 deletion src/libutil/filesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
# include <sys/stat.h>
# include <sys/types.h>
# include <unistd.h>
#ifdef __APPLE__
# include <utime.h>
#endif
#endif

#if defined(USE_STD_FILESYSTEM)
Expand Down Expand Up @@ -778,12 +781,19 @@ Filesystem::last_write_time(string_view path, std::time_t time) noexcept
times.modtime = time;
_wutime(u8path(path).c_str(), &times);
#else
#ifndef __APPLE__
struct timespec times[2];
times[0].tv_sec = 0;
times[0].tv_nsec = UTIME_OMIT;
times[1].tv_sec = time;
times[1].tv_nsec = 0;
utimensat((int)AT_FDCWD, u8path(path).c_str(), times, AT_SYMLINK_NOFOLLOW);
utimensat((int)AT_FDCWD, u8path(path).c_str(), times, AT_SYMLINK_NOFOLLOW);
#else
struct utimbuf times;
times.actime = time;
times.modtime = time;
utime(u8path(path).c_str(), &times);
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a bit of an awkward construction. Let's reverse the sense of this, so it's

#ifdef _WIN32
   ... windows case ...
#elif defined(__APPLE__)
   ... apple case ...
#else
    ... default unix case ...
#endif

I'm confused about why this is necessary. utimensat is POSIX, should be fine on Apple, and indeed on the Mac laptop I'm using this very moment, "man utimensat" shows me the right thing.

On the other hand, utime() is also POSIX, also exists on Linux, and is simpler, so I'm wondering if maybe the simpler solution is not to split apple from linux, and just use your new utime call for both? That is, maybe this is the best construct:

void
Filesystem::last_write_time(string_view path, std::time_t time) noexcept
{
#ifdef _WIN32
    struct _utimbuf times;
    times.actime  = time;
    times.modtime = time;
    _wutime(u8path(path).c_str(), &times);
#else
    struct utimbuf times;
    times.actime  = time;
    times.modtime = time;
    utime(u8path(path).c_str(), &times);
#endif
}

will that work for both Apple and Linux just fine?

#endif
}

Expand Down
2 changes: 1 addition & 1 deletion src/oiiotool/imagerec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,5 +413,5 @@ ImageRec::append_error(string_view message) const
&& "Accumulated error messages > 16MB. Try checking return codes!");
if (m_err.size() && m_err[m_err.size() - 1] != '\n')
m_err += '\n';
m_err += message;
m_err += std::string(message);
}