Re: msvcrt: fread: fill buffer on small reads

[Alexandre Julliard]

Markus Amsler  writes:
> - /* fill empty buffer on small reads */
> - if(!file->_cnt && rcnt <= MSVCRT_BUFSIZ) {
> - MSVCRT__filbuf(file);
> - /* reset internal buffer */
> - file->_cnt++;
> - file->_ptr = file->_base;
> - }

You need to handle errors properly, and MSVCRT__filbuf is probably not
the most appropriate thing to use here, a simple read would be
better.

msvcrt: fread: fill buffer on small reads

[Duane Clark]

Alexandre Julliard wrote:
> Markus Amsler  writes:
> > - /* fill empty buffer on small reads */
> > - if(!file->_cnt && rcnt <= MSVCRT_BUFSIZ) {
> > - MSVCRT__filbuf(file);
> > - /* reset internal buffer */
> > - file->_cnt++;
> > - file->_ptr = file->_base;
> > - }
>
> You need to handle errors properly, and MSVCRT__filbuf is probably not
> the most appropriate thing to use here, a simple read would be
> better.

Are you referring to _read() or read_i()? Those don't have an associated 
internal file buffer/cache (I guess because they don't have an 
associated file->_cnt and _ptr). Or were you referring to some other 
read call?

fread already does a _read() once it determines the current buffer is empty.

msvcrt: fread: fill buffer on small reads

[Alexandre Julliard]

Duane Clark  writes:
> Are you referring to _read() or read_i()? Those don't have an
> associated internal file buffer/cache (I guess because they don't have
> an associated file->_cnt and _ptr). Or were you referring to some
> other read call?
>
> fread already does a _read() once it determines the current buffer is empty.

Yes, and on small reads what it should do is a bigger _read() to
refill the buffer, instead of limiting it to the size that the app
requested.

msvcrt: fread: fill buffer on small reads

[Markus Amsler]

Duane Clark wrote:
> Alexandre Julliard wrote:
> > Markus Amsler  writes:
> > > - /* fill empty buffer on small reads */
> > > - if(!file->_cnt && rcnt <= MSVCRT_BUFSIZ) {
> > > - MSVCRT__filbuf(file);
> > > - /* reset internal buffer */
> > > - file->_cnt++;
> > > - file->_ptr = file->_base;
> > > - }
> >
> > You need to handle errors properly, and MSVCRT__filbuf is probably not
> > the most appropriate thing to use here, a simple read would be
> > better.
>
> Are you referring to _read() or read_i()? Those don't have an 
> associated internal file buffer/cache (I guess because they don't have 
> an associated file->_cnt and _ptr). Or were you referring to some 
> other read call?
>
> fread already does a _read() once it determines the current buffer is 
> empty.

I'm also not sure which read you mean. But i assumed some sort of 
stripped down inline MSVCRT__filbuf with read_i.

Markus

msvcrt: fread: fill buffer on small reads

[Duane Clark]

Markus Amsler wrote:
> Duane Clark wrote:
> > Alexandre Julliard wrote:
> > > Markus Amsler  writes:
> > > > - /* fill empty buffer on small reads */
> > > > - if(!file->_cnt && rcnt <= MSVCRT_BUFSIZ) {
> > > > - MSVCRT__filbuf(file);
> > > > - /* reset internal buffer */
> > > > - file->_cnt++;
> > > > - file->_ptr = file->_base;
> > > > - }
> > >
> > > You need to handle errors properly, and MSVCRT__filbuf is probably not
> > > the most appropriate thing to use here, a simple read would be
> > > better.
> >
> > Are you referring to _read() or read_i()? Those don't have an 
> > associated internal file buffer/cache (I guess because they don't have 
> > an associated file->_cnt and _ptr). Or were you referring to some 
> > other read call?
> >
> > fread already does a _read() once it determines the current buffer is 
> > empty.
>
> I'm also not sure which read you mean. But i assumed some sort of 
> stripped down inline MSVCRT__filbuf with read_i.

You probably want to continue to use _read(), since that handles the 
difference between text and binary file operations. Go ahead and take a 
shot at an implementation.

Make sure to run the file tests in the tests directory to verify that 
they all still pass.