PLEASE REVIEW: loader fix for gzipped kernels

2001-08-29 Thread John Polstra

I would appreciate another pair of eyes on the attached patch before
I commit it.

I have been working with gzipped kernels a lot lately, and have
noticed that when the loader tries to load certain kernels, it fails
with the message elf_loadexec: cannot seek.  I tracked this down to
a bug in src/lib/libstand/lseek.c, which is fixed by this patch.

Here is the bug that it fixed.  Libstand maintains a read-ahead buffer
for each open file, so that it can read in chunks of 512 bytes for
greater efficiency.  When the loader tries to lseek forward in a file
by a small amount, it sometimes happens that the target file offset is
already in the read-ahead buffer.  But the existing code in lseek.c
simply discards the contents of that buffer and does a seek directly
on the underlying file.  This results in an attempt to seek backwards
in the file, since some of the data has already been read into the
read-ahead buffer.  Gzipped data streams cannot seek backwards, so an
error is returned.

The code added by the patch checks to see if the desired file offset
is already in the read-ahead buffer.  If it is, the code simply
adjusts the buffer pointer and length, thereby avoiding a reverse seek
on the gzipped data stream.

The bug is present in both -current and -stable.  This patch is
relative to -stable, but it applies cleanly to -current too.

John
--
  John Polstra   [EMAIL PROTECTED]
  John D. Polstra  Co., Inc.Seattle, Washington USA
  Disappointment is a good sign of basic intelligence.  -- Chögyam Trungpa



Index: lseek.c
===
RCS file: /home/ncvs/src/lib/libstand/lseek.c,v
retrieving revision 1.1.1.1.6.1
diff -u -r1.1.1.1.6.1 lseek.c
--- lseek.c 2000/09/10 01:32:06 1.1.1.1.6.1
+++ lseek.c 2001/08/29 17:45:21
@@ -70,6 +70,8 @@
 off_t
 lseek(int fd, off_t offset, int where)
 {
+struct stat sb;
+off_t bufpos, filepos, target;
 struct open_file *f = files[fd];
 
 if ((unsigned)fd = SOPEN_MAX || f-f_flags == 0) {
@@ -94,6 +96,39 @@
return (-1);
}
return (f-f_offset);
+}
+
+/*
+ * If there is some unconsumed data in the readahead buffer and it
+ * contains the desired offset, simply adjust the buffer pointers.
+ */
+if (f-f_ralen != 0) {
+   if ((filepos = (f-f_ops-fo_seek)(f, (off_t)0, SEEK_CUR)) == -1)
+   return (-1);
+   bufpos = filepos - f-f_ralen;
+   switch (where) {
+   case SEEK_SET:
+   target = offset;
+   break;
+   case SEEK_CUR:
+   target = bufpos + offset;
+   break;
+   case SEEK_END:
+   if ((f-f_ops-fo_stat)(f, sb) == -1 || sb.st_size == -1) {
+   errno = EOFFSET;
+   return (-1);
+   }
+   target = sb.st_size + offset;
+   break;
+   default:
+   errno = EINVAL;
+   return (-1);
+   }
+   if (bufpos = target  target  filepos) {
+   f-f_raoffset += target - bufpos;
+   f-f_ralen -= target - bufpos;
+   return (target);
+   }
 }
 
 /*



Re: PLEASE REVIEW: loader fix for gzipped kernels

2001-08-29 Thread Garrett Rooney

On Wed, Aug 29, 2001 at 11:27:20AM -0700, John Polstra wrote:
 I would appreciate another pair of eyes on the attached patch before
 I commit it.
 
 I have been working with gzipped kernels a lot lately, and have
 noticed that when the loader tries to load certain kernels, it fails
 with the message elf_loadexec: cannot seek.  I tracked this down to
 a bug in src/lib/libstand/lseek.c, which is fixed by this patch.

so that's why the -CURRENT snapshot i was trying to install last night
refused to boot...  exactly that error.  damn that was irritating me.
i thought i was getting a corrupt iso image or something.

-- 
garrett rooney Unix was not designed to stop you from 
[EMAIL PROTECTED]   doing stupid things, because that would  
http://electricjellyfish.net/  stop you from doing clever things.

To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-hackers in the body of the message



RE: PLEASE REVIEW: loader fix for gzipped kernels

2001-08-29 Thread John Baldwin


On 29-Aug-01 John Polstra wrote:
 I would appreciate another pair of eyes on the attached patch before
 I commit it.

Looks good to me, but I'm only somewhat familiar with libstand. :)

-- 

John Baldwin [EMAIL PROTECTED] -- http://www.FreeBSD.org/~jhb/
PGP Key: http://www.baldwin.cx/~john/pgpkey.asc
Power Users Use the Power to Serve!  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-hackers in the body of the message



Re: PLEASE REVIEW: loader fix for gzipped kernels

2001-08-29 Thread John Polstra

In article [EMAIL PROTECTED],
John Baldwin  [EMAIL PROTECTED] wrote:
 
 Looks good to me, but I'm only somewhat familiar with libstand. :)

Thanks for taking a look at it.  Matt Dillon also reviewed it and gave
it a clean bill of health.  He made a suggestion for making the code a
bit smaller.  I'll incorporate that and then commit it to -current.

John

To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-hackers in the body of the message



Re: PLEASE REVIEW: loader fix for gzipped kernels

2001-08-29 Thread Matt Dillon


:In article [EMAIL PROTECTED],
:John Baldwin  [EMAIL PROTECTED] wrote:
: 
: Looks good to me, but I'm only somewhat familiar with libstand. :)
:
:Thanks for taking a look at it.  Matt Dillon also reviewed it and gave
:it a clean bill of health.  He made a suggestion for making the code a
:bit smaller.  I'll incorporate that and then commit it to -current.
:
:John

   I'll give it a quick test after you commit it (I can combine the test
   with some other work I'm doing).

-Matt

To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-hackers in the body of the message



Re: PLEASE REVIEW: loader fix for gzipped kernels

2001-08-29 Thread John Polstra

In article [EMAIL PROTECTED], Matt
Dillon [EMAIL PROTECTED] wrote:

I'll give it a quick test after you commit it (I can combine the
test with some other work I'm doing).

Thanks.  I've committed it, and it should hit the mirrors within the
next hour.  I tested it with both gzipped and full-size kernels, in
-current and -stable on the i386 and in -slightlystale on the Alpha.

John
-- 
  John Polstra   [EMAIL PROTECTED]
  John D. Polstra  Co., Inc.Seattle, Washington USA
  Disappointment is a good sign of basic intelligence.  -- Chögyam Trungpa


To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-hackers in the body of the message