Looks good to me, but I have some nitpicks below, on comments and stuff.

On Sat, Aug 29, 2020 at 7:44 AM Waldemar Kozaczuk <[email protected]>
wrote:

> Those 5 headers are mostly identical to their musl copies
> except they were modified to comment out *64 macros that serve
> as aliasing mechanism on musl side but conflicts in all
> places where we use LFS64 macro that uses weak_alias to provide
> symbol copies.
>
> So in order to avoid this collisions and replace the headers
> with symlinks we drop LFS64 in favor of aliases.ld mechanism
> which achieves the same goal.
>
> Signed-off-by: Waldemar Kozaczuk <[email protected]>
> ---
>  bsd/sys/cddl/compat/opensolaris/sys/stat.h |   1 +
>  fs/ramfs/ramfs_vnops.cc                    |   1 +
>  fs/vfs/main.cc                             |  21 +---
>  fs/vfs/vfs_vnode.cc                        |   1 +
>  include/api/sys/__stat.h                   |  14 +++
>  include/api/sys/mman.h                     |  56 +---------
>  include/api/sys/stat.h                     | 117 +--------------------
>  include/api/sys/statfs.h                   |  33 +-----
>  include/api/sys/statvfs.h                  |  58 +---------
>  include/api/sys/uio.h                      |  45 +-------
>  libc/aliases.ld                            |  15 ++-
>  libc/mman.cc                               |   5 -
>  12 files changed, 37 insertions(+), 330 deletions(-)
>  create mode 100644 include/api/sys/__stat.h
>  mode change 100644 => 120000 include/api/sys/mman.h
>  mode change 100644 => 120000 include/api/sys/stat.h
>  mode change 100644 => 120000 include/api/sys/statfs.h
>  mode change 100644 => 120000 include/api/sys/statvfs.h
>  mode change 100644 => 120000 include/api/sys/uio.h
>
> diff --git a/bsd/sys/cddl/compat/opensolaris/sys/stat.h
> b/bsd/sys/cddl/compat/opensolaris/sys/stat.h
> index 2d2f58fa..b33c6f31 100644
> --- a/bsd/sys/cddl/compat/opensolaris/sys/stat.h
> +++ b/bsd/sys/cddl/compat/opensolaris/sys/stat.h
> @@ -39,6 +39,7 @@
>  #ifndef _KERNEL
>  #include <sys/disk.h>
>
> +#undef fstat64
>

Yikes, this is ugly. But I guess we can live with that.

 static __inline int
>  fstat64(int fd, struct stat *sb)
>  {
> diff --git a/fs/ramfs/ramfs_vnops.cc b/fs/ramfs/ramfs_vnops.cc
> index 45591181..93866cc8 100644
> --- a/fs/ramfs/ramfs_vnops.cc
> +++ b/fs/ramfs/ramfs_vnops.cc
> @@ -32,6 +32,7 @@
>   */
>
>  #include <sys/stat.h>
> +#include <sys/__stat.h>
>

If we really need this new __stat.hh, please explain it in the commit
message.


>  #include <dirent.h>
>  #include <sys/param.h>
>
> diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc
> index 3c8b327b..b5c3a14a 100644
> --- a/fs/vfs/main.cc
> +++ b/fs/vfs/main.cc
> @@ -37,6 +37,7 @@
>  #include <sys/param.h>
>  #include <sys/statvfs.h>
>  #include <sys/stat.h>
> +#include <sys/__stat.h>
>  #include <sys/time.h>
>  #include <sys/sendfile.h>
>
> @@ -431,8 +432,6 @@ ssize_t preadv(int fd, const struct iovec *iov, int
> iovcnt, off_t offset)
>      return -1;
>  }
>
> -LFS64(preadv);
> -
>  ssize_t readv(int fd, const struct iovec *iov, int iovcnt)
>  {
>      return preadv(fd, iov, iovcnt, -1);
> @@ -466,7 +465,6 @@ ssize_t pwritev(int fd, const struct iovec *iov, int
> iovcnt, off_t offset)
>      errno = error;
>      return -1;
>  }
> -LFS64(pwritev);
>
>  ssize_t writev(int fd, const struct iovec *iov, int iovcnt)
>  {
> @@ -582,8 +580,6 @@ int fstat(int fd, struct stat *st)
>      return __fxstat(1, fd, st);
>  }
>
> -LFS64(fstat);
> -
>  extern "C"
>  int __fxstatat(int ver, int dirfd, const char *pathname, struct stat *st,
>          int flags)
> @@ -636,8 +632,6 @@ int fstatat(int dirfd, const char *path, struct stat
> *st, int flags)
>      return __fxstatat(1, dirfd, path, st, flags);
>  }
>
> -LFS64(fstatat);
> -
>  extern "C" int flock(int fd, int operation)
>  {
>      if (!fileref_from_fd(fd)) {
> @@ -1152,8 +1146,6 @@ int stat(const char *pathname, struct stat *st)
>      return __xstat(1, pathname, st);
>  }
>
> -LFS64(stat);
> -
>  TRACEPOINT(trace_vfs_lstat, "pathname=%s, stat=%p", const char*, struct
> stat*);
>  TRACEPOINT(trace_vfs_lstat_ret, "");
>  TRACEPOINT(trace_vfs_lstat_err, "errno=%d", int);
> @@ -1191,8 +1183,6 @@ int lstat(const char *pathname, struct stat *st)
>      return __lxstat(1, pathname, st);
>  }
>
> -LFS64(lstat);
> -
>  TRACEPOINT(trace_vfs_statfs, "\"%s\" %p", const char*, struct statfs*);
>  TRACEPOINT(trace_vfs_statfs_ret, "");
>  TRACEPOINT(trace_vfs_statfs_err, "%d", int);
> @@ -1221,8 +1211,6 @@ int __statfs(const char *pathname, struct statfs
> *buf)
>  }
>  weak_alias(__statfs, statfs);
>
> -LFS64(statfs);
> -
>  TRACEPOINT(trace_vfs_fstatfs, "\"%s\" %p", int, struct statfs*);
>  TRACEPOINT(trace_vfs_fstatfs_ret, "");
>  TRACEPOINT(trace_vfs_fstatfs_err, "%d", int);
> @@ -1253,8 +1241,6 @@ int __fstatfs(int fd, struct statfs *buf)
>  }
>  weak_alias(__fstatfs, fstatfs);
>
> -LFS64(fstatfs);
> -
>  static int
>  statfs_to_statvfs(struct statvfs *dst, struct statfs *src)
>  {
> @@ -1282,8 +1268,6 @@ statvfs(const char *pathname, struct statvfs *buf)
>      return statfs_to_statvfs(buf, &st);
>  }
>
> -LFS64(statvfs);
> -
>  int
>  fstatvfs(int fd, struct statvfs *buf)
>  {
> @@ -1294,9 +1278,6 @@ fstatvfs(int fd, struct statvfs *buf)
>      return statfs_to_statvfs(buf, &st);
>  }
>
> -LFS64(fstatvfs);
> -
> -
>  TRACEPOINT(trace_vfs_getcwd, "%p %d", char*, size_t);
>  TRACEPOINT(trace_vfs_getcwd_ret, "\"%s\"", const char*);
>  TRACEPOINT(trace_vfs_getcwd_err, "%d", int);
> diff --git a/fs/vfs/vfs_vnode.cc b/fs/vfs/vfs_vnode.cc
> index a292344f..8b307b83 100644
> --- a/fs/vfs/vfs_vnode.cc
> +++ b/fs/vfs/vfs_vnode.cc
> @@ -38,6 +38,7 @@
>  #include <stdio.h>
>  #include <errno.h>
>  #include <sys/stat.h>
> +#include <sys/__stat.h>
>
>  #include <osv/prex.h>
>  #include <osv/vnode.h>
> diff --git a/include/api/sys/__stat.h b/include/api/sys/__stat.h
> new file mode 100644
> index 00000000..cca00314
> --- /dev/null
> +++ b/include/api/sys/__stat.h
> @@ -0,0 +1,14 @@
> +/*
> + * Copyright (C) 2020 Waldemar Kozaczuk
> + *
> + * This work is open source software, licensed under the terms of the
> + * BSD license as described in the LICENSE file in the top-level
> directory.
> + */
> +
> +#ifndef OSV__SYS_STAT_H_
> +#define OSV__SYS_STAT_H_
> +
> +#define ALLPERMS (S_ISUID|S_ISGID|S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO)
> +#define S_BLKSIZE 512
>

Interesting. In Linux, these two macros are part of /usr/include/sys/stat.h.
I assume you added them to this new __stat.h because they were missing in
Musl?
Please add a comment in this file explaining this reason, and that the file
can be removed if Musl add them in a future version.


> +
> +#endif /* OSV__SYS_STAT_H_ */
> diff --git a/include/api/sys/mman.h b/include/api/sys/mman.h
> deleted file mode 100644
> index 06900199..00000000
> --- a/include/api/sys/mman.h
> +++ /dev/null
> @@ -1,55 +0,0 @@
> -#ifndef        _SYS_MMAN_H
> -#define        _SYS_MMAN_H
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
> -
> -#include <features.h>
> -
> -#define __NEED_mode_t
> -#define __NEED_size_t
> -#define __NEED_off_t
> -
> -#if defined(_GNU_SOURCE)
> -#define __NEED_ssize_t
> -#endif
> -
> -#include <bits/alltypes.h>
> -
> -#include <bits/mman.h>
> -
> -void *mmap (void *, size_t, int, int, int, off_t);
> -int munmap (void *, size_t);
> -
> -int mprotect (void *, size_t, int);
> -int msync (void *, size_t, int);
> -
> -int posix_madvise (void *, size_t, int);
> -
> -int mlock (const void *, size_t);
> -int munlock (const void *, size_t);
> -int mlockall (int);
> -int munlockall (void);
> -
> -#ifdef _GNU_SOURCE
> -void *mremap (void *, size_t, size_t, int, ...);
> -int remap_file_pages (void *, size_t, int, ssize_t, int);
> -#endif
> -
> -#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> -int madvise (void *, size_t, int);
> -int mincore (void *, size_t, unsigned char *);
> -#endif
> -
> -int shm_open (const char *, int, mode_t);
> -int shm_unlink (const char *);
> -
> -#if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
> -//#define mmap64 mmap
> -#define off64_t off_t
> -#endif
> -
> -#ifdef __cplusplus
> -}
> -#endif
> -#endif
> diff --git a/include/api/sys/mman.h b/include/api/sys/mman.h
> new file mode 120000
> index 00000000..93bc560f
> --- /dev/null
> +++ b/include/api/sys/mman.h
> @@ -0,0 +1 @@
> +../../../musl/include/sys/mman.h
> \ No newline at end of file
> diff --git a/include/api/sys/stat.h b/include/api/sys/stat.h
> deleted file mode 100644
> index 1026da4a..00000000
> --- a/include/api/sys/stat.h
> +++ /dev/null
> @@ -1,116 +0,0 @@
> -#ifndef        _SYS_STAT_H
> -#define        _SYS_STAT_H
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
> -
> -#include <features.h>
> -
> -#define __NEED_dev_t
> -#define __NEED_ino_t
> -#define __NEED_mode_t
> -#define __NEED_nlink_t
> -#define __NEED_uid_t
> -#define __NEED_gid_t
> -#define __NEED_off_t
> -#define __NEED_time_t
> -#define __NEED_blksize_t
> -#define __NEED_blkcnt_t
> -#define __NEED_struct_timespec
> -
> -#include <bits/alltypes.h>
> -
> -#include <bits/stat.h>
> -
> -#define st_atime st_atim.tv_sec
> -#define st_mtime st_mtim.tv_sec
> -#define st_ctime st_ctim.tv_sec
> -
> -#define S_IFMT  0170000
> -
> -#define S_IFDIR 0040000
> -#define S_IFCHR 0020000
> -#define S_IFBLK 0060000
> -#define S_IFREG 0100000
> -#define S_IFIFO 0010000
> -#define S_IFLNK 0120000
> -#define S_IFSOCK 0140000
> -
> -#define S_TYPEISMQ(buf)  0
> -#define S_TYPEISSEM(buf) 0
> -#define S_TYPEISSHM(buf) 0
> -#define S_TYPEISTMO(buf) 0
> -
> -#define S_ISDIR(mode)  (((mode) & S_IFMT) == S_IFDIR)
> -#define S_ISCHR(mode)  (((mode) & S_IFMT) == S_IFCHR)
> -#define S_ISBLK(mode)  (((mode) & S_IFMT) == S_IFBLK)
> -#define S_ISREG(mode)  (((mode) & S_IFMT) == S_IFREG)
> -#define S_ISFIFO(mode) (((mode) & S_IFMT) == S_IFIFO)
> -#define S_ISLNK(mode)  (((mode) & S_IFMT) == S_IFLNK)
> -#define S_ISSOCK(mode) (((mode) & S_IFMT) == S_IFSOCK)
> -
> -#ifndef S_IRUSR
> -#define S_ISUID 04000
> -#define S_ISGID 02000
> -#define S_ISVTX 01000
> -#define S_IRUSR 0400
> -#define S_IWUSR 0200
> -#define S_IXUSR 0100
> -#define S_IRWXU 0700
> -#define S_IRGRP 0040
> -#define S_IWGRP 0020
> -#define S_IXGRP 0010
> -#define S_IRWXG 0070
> -#define S_IROTH 0004
> -#define S_IWOTH 0002
> -#define S_IXOTH 0001
> -#define S_IRWXO 0007
> -#endif
> -
> -#define ALLPERMS (S_ISUID|S_ISGID|S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO)
> -
> -#define S_BLKSIZE 512
> -
> -#define UTIME_NOW  0x3fffffff
> -#define UTIME_OMIT 0x3ffffffe
> -
> -int stat(const char *__restrict, struct stat *__restrict);
> -int fstat(int, struct stat *);
> -int lstat(const char *__restrict, struct stat *__restrict);
> -int fstatat(int, const char *__restrict, struct stat *__restrict, int);
> -int chmod(const char *, mode_t);
> -int fchmod(int, mode_t);
> -int fchmodat(int, const char *, mode_t, int);
> -mode_t umask(mode_t);
> -int mkdir(const char *, mode_t);
> -int mknod(const char *, mode_t, dev_t);
> -int mkfifo(const char *, mode_t);
> -int mkdirat(int, const char *, mode_t);
> -int mknodat(int, const char *, mode_t, dev_t);
> -int mkfifoat(int, const char *, mode_t);
> -
> -int futimens(int, const struct timespec [2]);
> -int utimensat(int, const char *, const struct timespec [2], int);
> -
> -#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> -int lchmod(const char *, mode_t);
> -#define S_IREAD S_IRUSR
> -#define S_IWRITE S_IWUSR
> -#define S_IEXEC S_IXUSR
> -#endif
> -
> -#if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
> -//#define stat64 stat
> -//#define fstat64 fstat
> -//#define lstat64 lstat
> -//#define fstatat64 fstatat
> -#define blksize64_t blksize_t
> -#define blkcnt64_t blkcnt_t
> -#define ino64_t ino_t
> -#define off64_t off_t
> -#endif
> -
> -#ifdef __cplusplus
> -}
> -#endif
> -#endif
> diff --git a/include/api/sys/stat.h b/include/api/sys/stat.h
> new file mode 120000
> index 00000000..6a5dc2b1
> --- /dev/null
> +++ b/include/api/sys/stat.h
> @@ -0,0 +1 @@
> +../../../musl/include/sys/stat.h
> \ No newline at end of file
> diff --git a/include/api/sys/statfs.h b/include/api/sys/statfs.h
> deleted file mode 100644
> index 96ccb74b..00000000
> --- a/include/api/sys/statfs.h
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -#ifndef        _SYS_STATFS_H
> -#define        _SYS_STATFS_H
> -
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
> -
> -#include <features.h>
> -
> -#include <sys/statvfs.h>
> -
> -typedef struct {
> -       int __val[2];
> -} fsid_t;
> -
> -#include <bits/statfs.h>
> -
> -int statfs (const char *, struct statfs *);
> -int fstatfs (int, struct statfs *);
> -
> -#if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
> -//#define statfs64 statfs
> -//#define fstatfs64 fstatfs
> -#define fsblkcnt64_t fsblkcnt_t
> -#define fsfilcnt64_t fsfilcnt_t
> -#endif
> -
> -#ifdef __cplusplus
> -}
> -#endif
> -
> -#endif
> diff --git a/include/api/sys/statfs.h b/include/api/sys/statfs.h
> new file mode 120000
> index 00000000..2a5e8c75
> --- /dev/null
> +++ b/include/api/sys/statfs.h
> @@ -0,0 +1 @@
> +../../../musl/include/sys/statfs.h
> \ No newline at end of file
> diff --git a/include/api/sys/statvfs.h b/include/api/sys/statvfs.h
> deleted file mode 100644
> index f73d2e88..00000000
> --- a/include/api/sys/statvfs.h
> +++ /dev/null
> @@ -1,57 +0,0 @@
> -#ifndef        _SYS_STATVFS_H
> -#define        _SYS_STATVFS_H
> -
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
> -
> -#include <features.h>
> -
> -#define __NEED_fsblkcnt_t
> -#define __NEED_fsfilcnt_t
> -#include <bits/alltypes.h>
> -
> -#include <endian.h>
> -
> -struct statvfs {
> -       unsigned long f_bsize, f_frsize;
> -       fsblkcnt_t f_blocks, f_bfree, f_bavail;
> -       fsfilcnt_t f_files, f_ffree, f_favail;
> -#if __BYTE_ORDER == __LITTLE_ENDIAN
> -       unsigned long f_fsid;
> -       unsigned :8*(2*sizeof(int)-sizeof(long));
> -#else
> -       unsigned :8*(2*sizeof(int)-sizeof(long));
> -       unsigned long f_fsid;
> -#endif
> -       unsigned long f_flag, f_namemax;
> -       int __reserved[6];
> -};
> -
> -int statvfs (const char *__restrict, struct statvfs *__restrict);
> -int fstatvfs (int, struct statvfs *);
> -
> -#define ST_RDONLY 1
> -#define ST_NOSUID 2
> -#define ST_NODEV  4
> -#define ST_NOEXEC 8
> -#define ST_SYNCHRONOUS 16
> -#define ST_MANDLOCK    64
> -#define ST_WRITE       128
> -#define ST_APPEND      256
> -#define ST_IMMUTABLE   512
> -#define ST_NOATIME     1024
> -#define ST_NODIRATIME  2048
> -
> -#if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
> -//#define statvfs64 statvfs
> -//#define fstatvfs64 fstatvfs
> -#define fsblkcnt64_t fsblkcnt_t
> -#define fsfilcnt64_t fsfilcnt_t
> -#endif
> -
> -#ifdef __cplusplus
> -}
> -#endif
> -
> -#endif
> diff --git a/include/api/sys/statvfs.h b/include/api/sys/statvfs.h
> new file mode 120000
> index 00000000..281d48db
> --- /dev/null
> +++ b/include/api/sys/statvfs.h
> @@ -0,0 +1 @@
> +../../../musl/include/sys/statvfs.h
> \ No newline at end of file
> diff --git a/include/api/sys/uio.h b/include/api/sys/uio.h
> deleted file mode 100644
> index 5aba8926..00000000
> --- a/include/api/sys/uio.h
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -#ifndef _SYS_UIO_H
> -#define _SYS_UIO_H
> -
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
> -
> -#include <features.h>
> -
> -#define __NEED_size_t
> -#define __NEED_ssize_t
> -#define __NEED_struct_iovec
> -
> -#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> -#define __NEED_off_t
> -#endif
> -
> -#ifdef _GNU_SOURCE
> -#define __NEED_pid_t
> -#endif
> -
> -#include <bits/alltypes.h>
> -
> -ssize_t readv (int, const struct iovec *, int);
> -ssize_t writev (int, const struct iovec *, int);
> -
> -#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> -ssize_t preadv (int, const struct iovec *, int, off_t);
> -ssize_t pwritev (int, const struct iovec *, int, off_t);
> -#if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
> -#define off64_t off_t
> -#endif
> -#endif
> -
> -#ifdef _GNU_SOURCE
> -ssize_t process_vm_writev(pid_t, const struct iovec *, unsigned long,
> const struct iovec *, unsigned long, unsigned long);
> -ssize_t process_vm_readv(pid_t, const struct iovec *, unsigned long,
> const struct iovec *, unsigned long, unsigned long);
> -#endif
> -
> -#ifdef __cplusplus
> -}
> -#endif
> -
> -#endif
> diff --git a/include/api/sys/uio.h b/include/api/sys/uio.h
> new file mode 120000
> index 00000000..b909f7e4
> --- /dev/null
> +++ b/include/api/sys/uio.h
> @@ -0,0 +1 @@
> +../../../musl/include/sys/uio.h
> \ No newline at end of file
> diff --git a/libc/aliases.ld b/libc/aliases.ld
> index e2e2e73c..4ce216ad 100644
> --- a/libc/aliases.ld
> +++ b/libc/aliases.ld
> @@ -31,9 +31,10 @@ __setlocale = setlocale;
>  /* multibyte */
>  __mbrlen = mbrlen;
>
> -/* memory */
> +/* mman */
>  __munmap = munmap;
>  __mmap = mmap;
> +mmap64 = mmap;
>
>  /* stdio */
>  __dup3 = dup3;
> @@ -41,3 +42,15 @@ __dup3 = dup3;
>  /* string */
>  __strtok_r = strtok_r;
>  __strndup = strndup;
> +
> +/* vfs */
> +statfs64 = statfs;
> +fstatfs64 = fstatfs;
> +statvfs64 = statvfs;
> +fstatvfs64 = fstatvfs;
> +preadv64 = preadv;
> +pwritev64 = pwritev;
> +stat64 = stat;
> +fstat64 = fstat;
> +lstat64 = lstat;
> +fstatat64 = fstatat;
> diff --git a/libc/mman.cc b/libc/mman.cc
> index f0b8bc11..68877058 100644
> --- a/libc/mman.cc
> +++ b/libc/mman.cc
> @@ -185,11 +185,6 @@ void *mmap(void *addr, size_t length, int prot, int
> flags,
>      return ret;
>  }
>
> -extern "C" void *mmap64(void *addr, size_t length, int prot, int flags,
> -                      int fd, off64_t offset)
> -    __attribute__((alias("mmap")));
> -
> -
>  int munmap_validate(void *addr, size_t length)
>  {
>      if (!mmu::is_page_aligned(addr) || length == 0) {
> --
> 2.26.2
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/osv-dev/20200829044400.573969-1-jwkozaczuk%40gmail.com
> .
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/CANEVyjstjjQ14B3XiAbhZsw5f86unxRVntgoKn7Nf-ZKT63F6A%40mail.gmail.com.

Reply via email to