Opened 10 years ago

Closed 10 years ago

#3916 closed bug (fixed)

struct stat POSIX naming issue

Reported by: andreasf Owned by: bonefish
Priority: normal Milestone: R1
Component: System/libroot.so Version: R1/pre-alpha1
Keywords: Cc: planche2k@…
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

According to Open Group, struct stat has mandatory members st_mtim etc. http://www.opengroup.org/onlinepubs/9699919799/basedefs/sys_stat.h.html

In posix/sys/stat.h they are called st_mtime etc. instead. Notice the extra letter.

This breaks recent Git builds.

Is this POSIX deviation for BeOS compatibility, or did simply no one complain yet?

Attachments (1)

stat.h (4.6 KB ) - added by andreasf 10 years ago.
fixed stat.h

Download all attachments as: .zip

Change History (9)

by andreasf, 10 years ago

Attachment: stat.h added

fixed stat.h

comment:1 by andreasf, 10 years ago

I replaced time_t st_mtime with struct timespec st_mtim and added compatibility macro st_mtime st_mtim.tv_sec, as outlined above.

Makes Git happy there.

comment:2 by axeld, 10 years ago

Component: - GeneralSystem/libroot.so

This change would break binary compatibility, though. This change has been made in Issue 7 of the specs. Through the BeOS API detection, we could change this, but this would then require us to recompile all optional packages.

Wouldn't it be POSIX, I also wonder why they didn't adopt the BSD version of this, and called them st_mtimespec etc.

comment:3 by bonefish, 10 years ago

Owner: changed from axeld to bonefish
Status: newassigned

Since we generally support BeOS drivers, this change will also require API remapping in the kernel. Also, file systems should fill in the tv_nsec field correctly.

Will look into this later tonight.

comment:4 by bonefish, 10 years ago

Unfortunately this turns out to be seriously non-trivial, since the stat structure is not only passed to a handful of POSIX functions, but is also used in the Be API -- worst of all virtual methods like BRefFilter::Filter() and BStatable::GetStat(). ATM I don't see a solution other than forking off compatibility versions of the concerned libraries.

Suggestions welcome.

comment:5 by axeld, 10 years ago

I've probably overseen something, but can't we just call BRefFilter::Filter() with the stat as suggested by the binaries ABI version? And for GetStat(), we would use the "right" version of fstat() again depending on the ABI version of the caller?

The former might still be problematic in mixed environments (add-ons vs. app), but that shouldn't happen very often, anyway.

in reply to:  5 comment:6 by bonefish, 10 years ago

Replying to axeld:

I've probably overseen something, but can't we just call BRefFilter::Filter() with the stat as suggested by the binaries ABI version? And for GetStat(), we would use the "right" version of fstat() again depending on the ABI version of the caller?

The former might still be problematic in mixed environments (add-ons vs. app), but that shouldn't happen very often, anyway.

Not only add-ons would be a problem, but also libraries. And we don't control who calls BRefFilter::Filter() -- I believe it's only BFilePanel in the Be API, but an application or library could use the class for its own purposes as well. BStatable::GetStat() is really the lesser problem, since one can't even derive classes from BStatable.

A safe solution (besides compatibility libraries) would be to introduce a struct stat_beos which resembles the old struct stat, and use it in BRefFilter::Filter() and BStatable::GetStat(). In case of BRefFilter this would break source compatibility, in case of BStatable we could use a reserved virtual slot to prevent that, though.

comment:7 by andreasf, 10 years ago

Cc: planche2k@… added

comment:8 by bonefish, 10 years ago

Resolution: fixed
Status: assignedclosed

Implemented as I proposed in hrev30830.

Note: See TracTickets for help on using tickets.