Re: [2/2] msvcr90: Implement fread_s.

[Piotr Caban]

On 12/21/11 16:49, Kusanagi Kouichi wrote:
> +  if (bufsize / size<  count)
> +  {
> +    _invalid_parameter(NULL, NULL, NULL, 0, 0);
> +    fseek(fp, bufsize, SEEK_CUR);
> It's not correct to make a seek here. The file position changes you're 
> observing is related to data being read to FILE buffer.

This is also not working in case of small file. invalid_parameter 
handler should only be invoked if data is to big to fit into the buffer.
> +    /* fread_s raises an exeption if a file was opened by fopen */
> Probably you did something wrong here. It's allowed to use fread_s and 
> fopen (unless there was a broken version of msvcrt90).
> +    /* fclose raises an exception if a file was opened by fopen_s */
> This doesn't seem right as well.
> +    ret = p__fclose_nolock(file);
> It should be easy to add a proper implementation of _fclose_nolock.

Piotr

[2/2] msvcr90: Implement fread_s.

[Kusanagi Kouichi]

On 2011-12-21 17:34:49 +0100, Piotr Caban wrote:
> On 12/21/11 16:49, Kusanagi Kouichi wrote:
> > +  if (bufsize / size<  count)
> > +  {
> > +    _invalid_parameter(NULL, NULL, NULL, 0, 0);
> > +    fseek(fp, bufsize, SEEK_CUR);
> > It's not correct to make a seek here. The file position changes
> > you're observing is related to data being read to FILE buffer.
>
> This is also not working in case of small file. invalid_parameter
> handler should only be invoked if data is to big to fit into the
> buffer.

OK, I need more test.
> > +    /* fread_s raises an exeption if a file was opened by fopen */
> > Probably you did something wrong here. It's allowed to use fread_s
> > and fopen (unless there was a broken version of msvcrt90).
> > +    /* fclose raises an exception if a file was opened by fopen_s */
> > This doesn't seem right as well.

It might be my fault. I can mix them normally. But even if the test do only

fopen()
fread_s()

or

fopen_s()
fclose()

it still raises an exception.
> > +    ret = p__fclose_nolock(file);
> > It should be easy to add a proper implementation of _fclose_nolock.

It would be easy for you.

[2/2] msvcr90: Implement fread_s.

[Piotr Caban]

On 12/22/11 12:08, Kusanagi Kouichi wrote:
> It might be my fault. I can mix them normally. But even if the test do only
>
> fopen()
> fread_s()
> I've send a simple test to test bot. It doesn't crash there, it also 
> shows when read_s should succeed and fails with your implementation.
> https://testbot.winehq.org/JobDetails.pl?Key=16116
> > It should be easy to add a proper implementation of _fclose_nolock.
> > Something like this should work (it's a copy of implementation that is 
> > in msvcrt):
> > int CDECL _fclose_nolock(FILE *file)
> > {
> >      int r, flag;

flag = file->_flag;
     free(file->_tmpfname);
     file->_tmpfname = NULL;
     /* flush stdio buffers */
     if(file->_flag & _IOWRT)
         fflush(file);
     if(file->_flag & _IOMYBUF)
         free(file->_base);

     r=_close(file->_file);

     file->_flag = 0;

     return ((r == -1) || (flag & _IOERR) ? EOF : 0);
}

It's also not needed to check for functions existence in tests.