Re: [PATCH glibc] Add file record locking support
On Thu, 2015-01-08 at 18:03:31 +0100, Svante Signell wrote:
> On Thu, 2015-01-08 at 16:56 +0100, Guillem Jover wrote:
> > On Thu, 2015-01-08 at 12:40:12 +0100, Svante Signell wrote:
> > > Index: glibc-2.19/sysdeps/mach/hurd/fcntl.c
> > > ===================================================================
> > > --- glibc-2.19.orig/sysdeps/mach/hurd/fcntl.c
> > > +++ glibc-2.19/sysdeps/mach/hurd/fcntl.c
> > > @@ -128,56 +127,87 @@ __libc_fcntl (int fd, int cmd, ...)
> > > case F_SETLK:
> > > case F_SETLKW:
> > > {
> > […]
> > > struct flock *fl = va_arg (ap, struct flock *);
> > […]
> > > + struct flock64 *fl64 = malloc (sizeof (struct flock64));
> >
> > You are not checking if malloc failed, but in any case there's no need
> > at all to malloc the struct, just use «struct flock64 fl64».
>
> Yes you are right, no checks are made. I removed the malloc part. What
> about freeing fl64 later on?
You cannot free() memory from the stack, no. It gets released
automatically when it gets out of scope (but this is basic C).
> Index: glibc-2.19/sysdeps/mach/hurd/fcntl.c
> ===================================================================
> --- glibc-2.19.orig/sysdeps/mach/hurd/fcntl.c
> +++ glibc-2.19/sysdeps/mach/hurd/fcntl.c
> @@ -128,56 +127,87 @@ __libc_fcntl (int fd, int cmd, ...)
> + struct flock64 fl64;
> + fl64->l_type = fl->l_type;
> + fl64->l_whence = fl->l_whence;
> + fl64->l_start = fl->l_start;
> + fl64->l_len = fl->l_len;
> + fl64->l_pid = fl->l_pid;
> + err = HURD_FD_PORT_USE (d, __file_record_lock (port, cmd, fl64));
> + fl->l_type = fl64->l_type;
> + fl->l_whence = fl64->l_whence;
> + fl->l_start = fl64->l_start;
> + fl->l_len = fl64->l_len;
> + fl->l_pid = fl64->l_pid;
> + free (fl64);
> + result = err ? __hurd_dfail (fd, err) : 0;
> + break;
> + }
I'm assuming you didn't build this. It should be fl64.<member>, and
__file_record_lock(..., &fl64), and the free() would also have given
you an error there, please build-test it.
> + case F_GETLK64:
> + case F_SETLK64:
> + case F_SETLKW64:
> + {
> + struct flock64 *fl = va_arg (ap, struct flock64 *);
> +
> + switch (fl->l_type)
> + {
Still space vs tab here.
> + case F_RDLCK:
> + case F_WRLCK:
> + case F_UNLCK:
> + break;
> + default:
> + errno = EINVAL;
> + return -1;
> + break;
> }
Thanks,
Guillem
Reply to: