On Tue, 30 May 2023 19:33:03 +0800
zhangjial...@loongson.cn wrote:
+#ifdef __loongarch64
No.
Use the configuration mechanisms or whatever to determine the *actual
distinction you care about*, which is "does glibc have __xstat64", give
that a name, and use the name.
Some day, someone else will make a glibc that doesn't have __xstat64.
The change to adopt that should be one line in configure or the like,
not hundreds of changes scattered throughout the code.
+ int existed = (real_stat64(path, &buf) != -1);
+#else
int existed = (real___xstat64(_STAT_VER, path, &buf) != -1);
-
+#endif
This looks suspiciously similar to the logic you introduce elsewhere
for base_stat, base_fstat, etcetera.
Reproducing that up here:
+#ifdef __loongarch64
+#define base_fstatat(dirfd, path, buf, flags) real_fstatat64( dirfd,
path, buf, flags)
+#else
#define base_fstatat(dirfd, path, buf, flags)
real___fxstatat64(_STAT_VER, dirfd, path, buf, flags)
+#endif
Again, shouldn't be using a specific arch flag, should be using a
symbolic flag, but: Given this, maybe we should just be using
base_fstatat, base_stat, etcetera, in these lines, rather than
calling a specific real___foo function directly. Or alternatively,
we should be using pseudo_stat, etcetera, which also exist, and
I honestly don't remember why there are two sets of those.
So the change should look something like
- int existed = ((real___xstat64(_STAT_VER, path, &buf) != -1);
+ int existed = ((base_stat(path, &buf) != -1);
with suitable updates to the way we define the base_foo functions,
making them contingent on something like HAVE_XSTAT.
This would cover the vast majority of these changes, but not all of
them.
--- a/ports/linux/guts/fstat.c
+++ b/ports/linux/guts/fstat.c
@@ -7,8 +7,17 @@
* int fstat(int fd, struct stat *buf)
* int rc = -1;
*/
-
+#ifdef __loongarch64
+ struct stat64 buf64;
+ /* populate buffer with complete data */
+ real_fstat(fd, buf);
+ /* obtain fake data */
+ rc = wrap_fstat64(fd, &buf64);
+ /* overwrite */
+ pseudo_stat32_from64(buf, &buf64);
+#else
rc = wrap___fxstat(_STAT_VER, fd, buf);
+#endif
/* return rc;
* }
Here, we really do have a meaningful change; in the standard linux port,
we know that we can just forward fstat to __fxstat.
Probably the best model for the canonical way to fix this is to look
at the oldclone/newclone subports. When you have changes this
substantive, we don't want to do them with large inline #ifdefs; we want
to describe these as distinct subports, which have a different set of
functions. So in the hypothetical new "xstat" port, we'd have the
existing fstat and __fxstat wrappers, and in the new "noxstat" port,
we'd just have an fstat wrapper that uses the same logic.
diff --git a/templates/wrapfuncs.c b/templates/wrapfuncs.c
index 93bb671..14a42e2 100644
--- a/templates/wrapfuncs.c
+++ b/templates/wrapfuncs.c
@@ -13,7 +13,9 @@
* script if you want to modify this. */
@body
+#ifndef __loongarch64
static ${type} (*real_${name})(${decl_args}) = ${real_init};
+#endif
${maybe_skip}
I don't understand how this part can work at all; I was under the
impression that we actually did in fact use these real_foo objects,
and this seems to remove them all, so I don't even get how this
is compiling or running, because after this, there's no real_foo
function pointers at all?
Unfortunately, it turns out bitrot has taken my laptop and I can't
build pseudo on it natively because _STAT_VER no longer exists. So
I think the hypothetical distant future in which we conceivably
care about a system on which the real___xstatat(_STAT_VER, ...) calls
fail and isn't __longarch64 actually happened over a year ago and
we just didn't notice.
"Oops."
-s
