On Thursday 21 March 2019 at 10:47:26 +0100, Alexander Sverdlin wrote: > Hello Mike and all, > > On Thu, 15 Jan 2015 17:31:32 +0000 > Mike Crowe <[email protected]> wrote: > > > glibc's dprintf implementation does not work correctly with /dev/kmsg file > > descriptors because glibc treats receiving EBADF and EINVAL from lseek when > > trying to determine the current file position as errors. See > > https://sourceware.org/bugzilla/show_bug.cgi?id=17830 > > we need to conclude on this issue. This is a real bug which is ignored > for 4 years now. Mike, would you like to re-send a formal patch? > I can do it as well, preserving a link to your original patch/report. > In case you'd like to post it yourself, I can be a tester and/or > provide a reproducer.
Hi Alexander, I'm sorry for dropping the ball on this. I think I was travelling at the time. :( Thank you for picking it up. I don't mind rebasing my patch once I know which of my two solutions is preferred. It seems that the -ESPIPE one is the current favourite. Thanks. > > >>From what I can tell prior to Kay Sievers printk record commit > > e11fea92e13fb91c50bacca799a6131c81929986, calling lseek(fd, 0, SEEK_CUR) > > with such a file descriptor would not return an error. > > > > Prior to Kay's change, Arnd Bergmann's commit > > 6038f373a3dc1f1c26496e60b6c40b164716f07e seemed to go to some lengths to > > preserve the successful return code rather than returning (the perhaps more > > logical) -ESPIPE. > > > > glibc is happy with either a successful return or -ESPIPE. > > > > For maximum compatibility it seems that success should be returned but > > given Kay's new seek interface perhaps this isn't helpful. > > > > This patch ensures that such a seek succeeds: > > > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > > index 02d6b6d..b3ff6f0 100644 > > --- a/kernel/printk/printk.c > > +++ b/kernel/printk/printk.c > > @@ -693,7 +693,7 @@ static loff_t devkmsg_llseek(struct file *file, loff_t > > offset, int whence) > > loff_t ret = 0; > > > > if (!user) > > - return -EBADF; > > + return (whence == SEEK_CUR) ? 0 : -EBADF; > > if (offset) > > return -ESPIPE; > > > > @@ -718,6 +718,11 @@ static loff_t devkmsg_llseek(struct file *file, loff_t > > offset, int whence) > > user->idx = log_next_idx; > > user->seq = log_next_seq; > > break; > > + case SEEK_CUR: > > + /* For compatibility with userspace requesting the > > + * current file position. */ > > + ret = 0; > > + break; > > default: > > ret = -EINVAL; > > } > > > > (although it could be argued that the !user case should return -ESPIPE > > rather than EBADF since the file descriptor _is_ valid.) > > > > and this patch causes a failure that glibc is prepared to accept: > > > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > > index 02d6b6d..f6b0c93 100644 > > --- a/kernel/printk/printk.c > > +++ b/kernel/printk/printk.c > > @@ -693,7 +693,7 @@ static loff_t devkmsg_llseek(struct file *file, loff_t > > offset, int whence) > > loff_t ret = 0; > > > > if (!user) > > - return -EBADF; > > + return -ESPIPE; > > if (offset) > > return -ESPIPE; > > > > @@ -718,6 +718,11 @@ static loff_t devkmsg_llseek(struct file *file, loff_t > > offset, int whence) > > user->idx = log_next_idx; > > user->seq = log_next_seq; > > break; > > + case SEEK_CUR: > > + /* For compatibility with userspace expecting SEEK_CUR > > + * to not yield EINVAL. */ > > + ret = -ESPIPE; > > + break; > > default: > > ret = -EINVAL; > > } > > > > Either makes dprintf work, but is either the right solution? Mike.
