Solar Designer wrote:
> 
> On Tue, Apr 23, 2002 at 12:09:14PM +0100, Ben Laurie wrote:
> > Solar Designer wrote:
> > > This sounds like a bug to me.  Noticed it last year and I've just
> > > checked that it's still not resolved in the latest snapshot.
> > >
> > > jill!solar:~/build/openssl-SNAP-20020416$ apps/openssl dgst -md5 /bin/ls
> > > MD5(/bin/ls)= d93498d9f52c3dc0330ab930fe3ffc50
> > >
> > > OK.
> > >
> > > jill!solar:~/build/openssl-SNAP-20020416$ apps/openssl dgst -md5 /bin
> > > MD5(/bin)= d41d8cd98f00b204e9800998ecf8427e
> > > jill!solar:~/build/openssl-SNAP-20020416$ apps/openssl dgst -md5 /dev/log
> > > MD5(/dev/log)= d41d8cd98f00b204e9800998ecf8427e
> > >
> > > Wrong.  I'd want it to fail with a message to stderr and a non-zero
> > > exit code.  Also on any read error, not just on non-regular files.
> > >
> > > open("/bin", O_RDONLY)                  = 4
> > > [some getpid()'s]
> > > read(4, 0x8189fb0, 8192)                = -1 EISDIR (Is a directory)
> > > [lots of getpid()'s, why?!]
> > > write(1, "MD5(/bin)= d41d8cd98f00b204e9800"..., 44) = 44
> >
> > Try this for size (against 0.9.7, but probably will work on other
> > versions)....
> 
> Applied to the same snapshot as above with "patch -l" (probably tabs
> got converted to spaces somewhere in your mail).
> 
> It's better but still not perfect:
> 
> jill!solar:~/build/openssl-SNAP-20020416$ apps/openssl dgst -md5 /dev/log
> Read Error
> MD5(/dev/log)= jill!solar:~/build/openssl-SNAP-20020416$ echo $?
> 0
> 
> I'd rather not have it output the "MD5(/dev/log)= " until it has the
> hash value and have it exit non-zero.

Yeah, I agree. I'm looking at it again.

> >                 ret=fread(out,1,(int)outl,(FILE *)b->ptr);
> > +               if(ret == 0 && ferror((FILE *)b->ptr))
> > +                       ret=-1;
> 
> I am looking at this out of context (so I don't know if the fread()
> will always be re-called when "ret" is smaller than the requested
> "outl") but I'd change the "ret == 0" to "ret < outl" (possibly with
> a cast) or drop this check entirely (the ferror() alone is sufficient).

The read isn't retried, so I'd suggest that you want to return read data
and return the error on the next call (which should read no data). i.e.
no change.

> 
> size_t fread(void *ptr, size_t size, size_t nitems, FILE *stream);
> 
>      Upon successful completion, fread() returns the number of members
>      successfully read which is less than nitems only if a read error or
>      end-of-file is encountered. If size or nitems is 0, fread() returns 0
>      and the contents of the array and the state of the stream remain
>      unchanged. Otherwise, if a read error occurs, the error indicator for
>      the stream is set and errno is set to indicate the error.
> 
> (SUSv2)
> 
> > --- apps/dgst.c 2002/04/06 18:59:57     1.23.2.2
> > +++ apps/dgst.c 2002/04/23 11:06:14
> > @@ -362,7 +362,13 @@
> >         for (;;)
> >                 {
> >                 i=BIO_read(bp,(char *)buf,BUFSIZE);
> > -               if (i <= 0) break;
> > +               if(i < 0)
> > +                   {
> > +                       BIO_printf(bio_err, "Read Error\n");
> > +                       ERR_print_errors(bio_err);
> > +                       return;
> > +                       }
> > +               if (i == 0) break;
> >                 }
> 
> I'm not sure how this would handle partial reads.  I don't know the
> semantics of BIO_read(), but it appears weird to retry with the same
> arguments on any positive return value.

This is the weird world of BIOs - the BIO we're using is actually one
that does an MD5 on stuff read through it, so we aren't really retrying,
just sucking in the whole file and digesting it...

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html       http://www.thebunker.net/

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       [EMAIL PROTECTED]
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to