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.

Sign up for GitHub

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

Jump to bottom

Fix build failure in Alpha architecture #2986

Closed
wants to merge 1 commit into from
Closed

Fix build failure in Alpha architecture #2986

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion programs/util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ int UTIL_utime(const char* filename, const stat_t *statbuf)
* that struct stat has a struct timespec st_mtim member. We need this
* check because there are some platforms that claim to be POSIX 2008
* compliant but which do not have st_mtim... */
#if (PLATFORM_POSIX_VERSION >= 200809L) && defined(st_mtime)
#if (PLATFORM_POSIX_VERSION >= 200809L) && defined(st_mtime) || defined(__alpha__)
Copy link
Contributor

Choose a reason for hiding this comment

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

So I guess it means __alpha__ platform never claims POSIX 2008 compliance, since it probably is not,
yet nonetheless support struct timespec and st_mtime as extended features ?

The surprising part here is that, in this construction, __alpha__ is presumed enough of a hint to guarantee the support of struct timespec, no matter the software level. I would have expected __alpha__ to be much older than struct timespec, and therefore logically, older variants of software for __alpha__ would not provide struct timespec. Though I don't know enough about __alpha__ history to be sure about that.

Another avenue could have been to declare PLATFORM_POSIX_VERSION = 200809L when the right set of conditions is present (__alpha__ and maybe another condition), but this would imply support of other stuff beyond struct timespec.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about :

#if (PLATFORM_POSIX_VERSION >= 200809L || defined(__alpha__)) && defined(st_mtime)

Copy link
Contributor

Choose a reason for hiding this comment

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

@Cyan4973, as the linked bug states, Alpha does not define st_mtime (which makes it non-compliant with POSIX...). So that won't work.

Copy link
Contributor

Choose a reason for hiding this comment

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

okay,
so st_mtime is in fact not required,
it's just employed as a way to improve odds that struct timespec.st_mtim is actually supported ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct. We previously found some platform (I forget which) which claims to be POSIX 2008 but doesn't have struct timespec and we detect that by observing that they don't define st_mtime.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can you add parenthesis to clarify the statement?

Suggested change
#if (PLATFORM_POSIX_VERSION >= 200809L) && defined(st_mtime) || defined(__alpha__)
#if ((PLATFORM_POSIX_VERSION >= 200809L) && defined(st_mtime)) || defined(__alpha__)

/* (atime, mtime) */
struct timespec timebuf[2] = { {0, UTIME_NOW} };
timebuf[1] = statbuf->st_mtim;
Expand Down