Hello, Svante Signell, le sam. 22 déc. 2018 13:07:37 +0100, a ecrit: > diskfs_S_file_lock (struct protid *cred, int flags) > { ... > + /* XXX: Fix for flock(2) calling fcntl(2) */ > + if ((openstat & O_RDONLY) && !(openstat & O_WRONLY)) openstat |= O_WRONLY; > + if (!(openstat & O_RDONLY) && (openstat & O_WRONLY)) openstat |= O_RDONLY;
I don't understand this: in which application case is this needed? > +error_t > +diskfs_S_file_record_lock (struct protid *cred, > + int cmd, > + struct flock64 *lock, > + mach_port_t rendezvous) > +{ > + struct node *node; > + error_t err; > + > + if (! cred) > + return EOPNOTSUPP; > + > + /* XXX: Call with non-null for per process locking*/ > + rendezvous = MACH_PORT_NULL; Why setting it to MACH_PORT_NULL? AIUI, we can just pass the rendezvous parameter as it is to fshelp_rlock_tweak (and currently it'll already be MACH_PORT_NULL), and when we'll implement proper support it'll be in fshelp_rlock_tweak, we won't have to do anything here. And similarly in other translator libs. > + node = cred->po->np; > + pthread_mutex_lock (&node->lock); > + err = fshelp_rlock_tweak (&node->userlock, &node->lock, > + &cred->po->lock_status, cred->po->openstat, > + node->dn_stat.st_size, cred->po->filepointer, > + cmd, lock, rendezvous); > + pthread_mutex_unlock (&node->lock); > + return err; > +} > libfshelp/ChangeLog > 2018-12-07 Svante Signell <svante.sign...@gmail.com> > > * Update copyright years. > > * rlock-tweak.c(fshelp-rlock_tweak): Please take care of typos, it's important for being able to grep etc. > Add new argument: > mach_port_t rendezvous. Calls using MACH_PORT_NULL indicates > per opened file locking. Calls with !MACH_PORT_NULL indicates > per process locking. l_pid is set to -1 when a conflicting > lock is taken by another process, like BSD does. This and per > process locking will be fixed by new proc RPCs implementing > proc_{server,user}_identify. Such information does not belong to the ChangeLog, but to the .h file. > +# Define the 64 bit versions of the second argument to fcntl() > +# Can safely be removed when glibc is updated > +EXTRA_CPP_FLAGS= -DF_GETLK64=10 -DF_SETLK64=11 -DF_SETLKW64=12 > +rlock-tweak-CPPFLAGS += $(EXTRA_CPP_FLAGS) Avoid such hooks in your patches. Yes, when adding an RPC the bootstrap process needs some tweaks, but that's not what we will commit in the end. > Index: hurd-0.9.git20181030-3.3/libfshelp/rlock.h > =================================================================== > --- /dev/null > +++ hurd-0.9.git20181030-3.3/libfshelp/rlock.h > @@ -0,0 +1,111 @@ ... > + > +/* > +libthreads/cthreads.h: > +typedef struct condition { > + spin_lock_t lock; > + struct cthread_queue queue; > + const char *name; <-- differs > + struct cond_imp *implications; > +} *condition_t; > +rlock.h: > + struct condition wait; > +here replaced by > + struct pthread_cond_t wait; > +/usr/include/pthread/pthreadtypes.h:typedef struct __pthread_cond > pthread_cond_t; > +/usr/include/i386-gnu/bits/condition.h:struct __pthread_cond > +struct __pthread_cond > +{ > + __pthread_spinlock_t __lock; > + struct __pthread *__queue; > + struct __pthread_condattr *__attr; <-- differs > + struct __pthread_condimpl *__impl; > + void *__data; <-- differs > +}; > + */ Drop this as well, since we won't commit it. > Index: hurd-0.9.git20181030-3.3/pflocal/fs.c > =================================================================== > --- hurd-0.9.git20181030-3.3.orig/pflocal/fs.c > +++ hurd-0.9.git20181030-3.3/pflocal/fs.c > @@ -39,3 +37,11 @@ S_file_check_access (struct sock_user *c > > return 0; > } > + > +error_t > +S_file_record_lock (struct sock_user *cred, int cmd, > +//S_file_record_lock (file_t file, int cmd, > + flock_t *flock64, mach_port_t rendezvous) Again, do not keep such commented code which we won't commit. > Index: hurd-0.9.git20181030-3.3/libnetfs/make-peropen.c > =================================================================== > --- hurd-0.9.git20181030-3.3.orig/libnetfs/make-peropen.c > +++ hurd-0.9.git20181030-3.3/libnetfs/make-peropen.c > struct peropen * > netfs_make_peropen (struct node *np, int flags, struct peropen *context) > { > + error_t err; > struct peropen *po = malloc (sizeof (struct peropen)); > > if (!po) > return NULL; > > po->filepointer = 0; > - po->lock_status = LOCK_UN; > + err = fshelp_rlock_po_init (&po->lock_status); > + if (err) > + return NULL; > + /* XXX: refcount_init() cannot be used since reference counting > + starts at 0 not 1 as for libdiskfs */ > refcount_init (&po->refcnt, 1); I don't understand the XXX: how is refcount related to rlocks? Svante Signell, le sam. 22 déc. 2018 13:09:26 +0100, a ecrit: > Note that the following glibc patches have been commented out from the > series file: > #hurd-i386/git-fcntl64.diff > #hurd-i386/git-lockf-0.diff > #hurd-i386/tg-WRLCK-upgrade.diff And thus I'll have to do the rebase myself? That'll mean I'll have to defer applying the patches to when I get the time to do such rebase... Unless you do the rebase so I have patches that I can just apply. Really, remember that the more you prepare the patches, the earlier I will find the time to apply them. If they can't be applied as such and I have to do some work on them, it means I'll have to defer to when I get the time to do it. > Index: glibc-2.28-3.1/sysdeps/mach/hurd/fcntl.c > =================================================================== > --- glibc-2.28-3.1.orig/sysdeps/mach/hurd/fcntl.c > +++ glibc-2.28-3.1/sysdeps/mach/hurd/fcntl.c > @@ -28,6 +27,7 @@ __libc_fcntl (int fd, int cmd, ...) > { > va_list ap; > struct hurd_fd *d; > + mach_port_t rendezvous = MACH_PORT_NULL; > int result; > > d = _hurd_fd_get (fd); Avoid introducing the variable for the whole function when you actually need it only for a couple of cases. Thanks Samuel