Apologies for delay, such simple thing should have been sent long ago.
Joakim, please, confirm.
-------------------------------------------
[PATCH] seq_file: make seq_lseek accept SEEK_END
and pretend seq_files have zero length. This should be enough
to fix busybox start-stop-daemon:
http://marc.info/?t=120836691600002&r=1&w=2
It does xlseek(fd, 0, SEEK_END) to estimate amount of memory to malloc
but satisfied with 0. Sudden -EINVAL from lseek(2) breaks it.
X-Introduced-By: f16278c679aa72e28288435b313ba2d4494d6be5
Signed-off-by: Alexey Dobriyan <[email protected]>
---
fs/seq_file.c | 2 ++
1 file changed, 2 insertions(+)
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -254,6 +254,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int origin)
switch (origin) {
case 1:
offset += file->f_pos;
+ case 2:
+ /* pretend it's zero length */
case 0:
if (offset < 0)
break;
busybox has been fixed, at least in trunk, not to do
xlseek(fd, 0, SEEK_END) anymore to address this problem.
Perhaps there are other apps out there that needs this too, but I don't
know of any. Older busyboxes needs this change though.
I have moved on and don't have a setup handy where I can test this.
Perhaps someone at the bb list has, CC:ing bb list.
Jocke
On Mon, 2008-06-09 at 15:01 +0400, Alexey Dobriyan wrote:
> Apologies for delay, such simple thing should have been sent long ago.
> Joakim, please, confirm.
> -------------------------------------------
> [PATCH] seq_file: make seq_lseek accept SEEK_END
>
> and pretend seq_files have zero length. This should be enough
> to fix busybox start-stop-daemon:
> http://marc.info/?t=120836691600002&r=1&w=2
>
> It does xlseek(fd, 0, SEEK_END) to estimate amount of memory to malloc
> but satisfied with 0. Sudden -EINVAL from lseek(2) breaks it.
>
> X-Introduced-By: f16278c679aa72e28288435b313ba2d4494d6be5
> Signed-off-by: Alexey Dobriyan <[email protected]>
> ---
>
> fs/seq_file.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -254,6 +254,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int origin)
> switch (origin) {
> case 1:
> offset += file->f_pos;
> + case 2:
> + /* pretend it's zero length */
> case 0:
> if (offset < 0)
> break;
>
>
On Mon, 09 Jun 2008 13:52:55 +0200
Joakim Tjernlund <[email protected]> wrote:
> busybox has been fixed, at least in trunk, not to do
> xlseek(fd, 0, SEEK_END) anymore to address this problem.
> Perhaps there are other apps out there that needs this too, but I don't
> know of any. Older busyboxes needs this change though.
> I have moved on and don't have a setup handy where I can test this.
> Perhaps someone at the bb list has, CC:ing bb list.
>
Nevertheless it'd be nice to fix old versions.
To some extent that depends on when we broke it. See below.
>
> On Mon, 2008-06-09 at 15:01 +0400, Alexey Dobriyan wrote:
> > Apologies for delay, such simple thing should have been sent long ago.
> > Joakim, please, confirm.
> > -------------------------------------------
> > [PATCH] seq_file: make seq_lseek accept SEEK_END
> >
> > and pretend seq_files have zero length. This should be enough
> > to fix busybox start-stop-daemon:
> > http://marc.info/?t=120836691600002&r=1&w=2
> >
> > It does xlseek(fd, 0, SEEK_END) to estimate amount of memory to malloc
> > but satisfied with 0. Sudden -EINVAL from lseek(2) breaks it.
> >
> > X-Introduced-By: f16278c679aa72e28288435b313ba2d4494d6be5
> > Signed-off-by: Alexey Dobriyan <[email protected]>
> > ---
> >
> > fs/seq_file.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > --- a/fs/seq_file.c
> > +++ b/fs/seq_file.c
> > @@ -254,6 +254,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int origin)
> > switch (origin) {
> > case 1:
> > offset += file->f_pos;
> > + case 2:
> > + /* pretend it's zero length */
> > case 0:
> > if (offset < 0)
> > break;
> >
> >
Is returning zero logical, given what we do with SEEK_SET?
afait we _could_ just do
offset = <largenum>
/* fall through */
and let the SEEK_SET code do its thing. But I might have misread it.
But still, it looks like it'd be possible to return a true(ish) value
from SEEK_END?
> > X-Introduced-By: f16278c679aa72e28288435b313ba2d4494d6be5
Please don't do that. Instead, quote the commit ID _and_ the first few
lines of the commit itself, like:
commit f16278c679aa72e28288435b313ba2d4494d6be5
Author: Hans Rosenfeld <[email protected]>
Date: Fri Mar 21 18:46:59 2008 -0500
Change pagemap output format to allow for future reporting of huge pages
And here we see one reason: that commit cannot be the one which
introduced this problem.
On Wed, Jun 11, 2008 at 02:29:09PM -0700, Andrew Morton wrote:
> On Mon, 09 Jun 2008 13:52:55 +0200
> Joakim Tjernlund <[email protected]> wrote:
>
> > busybox has been fixed, at least in trunk, not to do
> > xlseek(fd, 0, SEEK_END) anymore to address this problem.
> > Perhaps there are other apps out there that needs this too, but I don't
> > know of any. Older busyboxes needs this change though.
> > I have moved on and don't have a setup handy where I can test this.
> > Perhaps someone at the bb list has, CC:ing bb list.
> >
>
> Nevertheless it'd be nice to fix old versions.
>
> To some extent that depends on when we broke it. See below.
>
> >
> > On Mon, 2008-06-09 at 15:01 +0400, Alexey Dobriyan wrote:
> > > Apologies for delay, such simple thing should have been sent long ago.
> > > Joakim, please, confirm.
> > > -------------------------------------------
> > > [PATCH] seq_file: make seq_lseek accept SEEK_END
> > >
> > > and pretend seq_files have zero length. This should be enough
> > > to fix busybox start-stop-daemon:
> > > http://marc.info/?t=120836691600002&r=1&w=2
> > >
> > > It does xlseek(fd, 0, SEEK_END) to estimate amount of memory to malloc
> > > but satisfied with 0. Sudden -EINVAL from lseek(2) breaks it.
> > >
> > > X-Introduced-By: f16278c679aa72e28288435b313ba2d4494d6be5
> > > Signed-off-by: Alexey Dobriyan <[email protected]>
> > > ---
> > >
> > > fs/seq_file.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > --- a/fs/seq_file.c
> > > +++ b/fs/seq_file.c
> > > @@ -254,6 +254,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int origin)
> > > switch (origin) {
> > > case 1:
> > > offset += file->f_pos;
> > > + case 2:
> > > + /* pretend it's zero length */
> > > case 0:
> > > if (offset < 0)
> > > break;
> > >
> > >
>
> Is returning zero logical, given what we do with SEEK_SET?
Definitely illogical. :-)
> afait we _could_ just do
>
> offset = <largenum>
> /* fall through */
>
> and let the SEEK_SET code do its thing. But I might have misread it.
If it's largenum, not enough output will be generated during each
iteration until kmalloc() will refuse to give more memory.
> But still, it looks like it'd be possible to return a true(ish) value
> from SEEK_END?
I think this is doable.
On the other side is
a) ugliness (remember how much output each item generates, remember ->index,
allocate memory to do that),
b) SEEK_END reported and reports zero-length for other proc files
(and sysctls), and nobody really cared.