[PATCH v2] msvcirt: Implement ifstream.

Piotr Caban piotr at codeweavers.com
Tue Sep 1 10:54:45 CDT 2020


Hi Arek,

The patch is quite long so I might have missed something.

> +/* ??0ifstream@@QAE at HPADH@Z */
> +/* ??0ifstream@@QEAA at HPEADH@Z */
> +DEFINE_THISCALL_WRAPPER(ifstream_buffer_ctor, 20)
> +istream* __thiscall ifstream_buffer_ctor(istream *this, filedesc fd, char *buffer, int length, BOOL virt_init)
> +{
> +    ios *base;
> +    filebuf *fb = MSVCRT_operator_new(sizeof(filebuf));
> +
> +    TRACE("(%p %d %p %d %d)\n", this, fd, buffer, length, virt_init);
> +
> +    if (fb)
> +    {
> +        filebuf_fd_reserve_ctor(fb, fd, buffer, length);
> +        istream_sb_ctor(this, &fb->base, virt_init);
> +    }
> +    else
> +        istream_ctor(this, virt_init);
The allocation failure handling looks quite strange. It's probably 
better to remove it so the application crashes instead of misbehaving.

> +/* ??0ifstream@@QAE at PBDHH@Z */
> +/* ??0ifstream@@QEAA at PEBDHH@Z */
> +DEFINE_THISCALL_WRAPPER(ifstream_open_ctor, 20)
> +istream* __thiscall ifstream_open_ctor(istream *this, const char *name, ios_open_mode mode, int protection, BOOL virt_init)
> +{
> +    ios *base;
> +    filebuf *fb = MSVCRT_operator_new(sizeof(filebuf));
> +
> +    TRACE("(%p %s %d %d %d)\n", this, name, mode, protection, virt_init);
> +
> +    if (fb)
> +    {
> +        filebuf_ctor(fb);
> +        istream_sb_ctor(this, &fb->base, virt_init);
> +        filebuf_open(fb, name, mode, protection);
We should probably add OPENMODE_in as in e.g. 
msvcp90/ios.c:basic_ifstream_char_open. The same applies to all 
functions with mode argument.

> +/* ?attach at ifstream@@QAEXH at Z */
> +/* ?attach at ifstream@@QEAAXH at Z */
> +DEFINE_THISCALL_WRAPPER(ifstream_attach, 8)
> +void __thiscall ifstream_attach(istream *this, filedesc fd)
> +{
> +    TRACE("(%p %d)\n", this, fd);
> +    filebuf_attach(ifstream_rdbuf(this), fd);
> +}
You should probably set failbit if filebuf_attach fails. The same 
applies to other functions on failure.

> +/* ?close at ifstream@@QAEXXZ */
> +/* ?close at ifstream@@QEAAXXZ */
> +DEFINE_THISCALL_WRAPPER(ifstream_close, 4)
> +void __thiscall ifstream_close(istream *this)
> +{
> +    TRACE("(%p)\n", this);
> +    filebuf_close(ifstream_rdbuf(this));
> +}
You should probably set failbit if filebuf_close fails. Also you should 
probably clear errors on successful close.


> +    pifs = call_func5(p_ifstream_open_ctor, &ifs, filename, OPENMODE_in, filebuf_openprot, TRUE);
> +    pfb = (filebuf*) ifs.base_ios.sb;
> +    ok(ifs.base_ios.delbuf == 1, "expected 1 got %d\n", ifs.base_ios.delbuf);
> +    ok(pifs == &ifs, "wrong return, expected %p got %p\n", &ifs, pifs);
> +    ok(pfb->base.allocated == 1, "wrong allocate value, expected 1 got %d\n", pfb->base.allocated);
> +    ok(pfb->base.unbuffered == 0, "wrong unbuffered value, expected 0 got %d\n", pfb->base.unbuffered);
> +    ok(pfb->base.base != NULL, "wrong buffer, expected not %p got %p\n", NULL, pfb->base.base);
> +    ok(pfb->base.ebuf != NULL, "wrong ebuf, expected not %p got %p\n", NULL, pfb->base.ebuf);
> +    ok(pfb->fd != -1, "wrong fd, expected not -1 got %d\n", pfb->fd);
> +    fd = pfb->fd;
> +    ok(pfb->close == 1, "wrong value, expected 1 got %d\n", pfb->close);
> +    call_func1(p_ifstream_vbase_dtor, &ifs);
> +    ok(_close(fd) == -1, "expected ifstream to close opened file\n");
Isn't the fd closed by ifstream_vbase_dtor -> filebuf_dtor -> filebuf_close?

> +    /* setbuf - seems to be a nop and always return NULL */
> +    pifs = call_func5(p_ifstream_buffer_ctor, &ifs, 64, NULL, 0, TRUE);
Passing arbitrary value as fd doesn't look right. How about changing 64 
to -1? It will also show some problems with setbuf implementation.

> +    psb = call_func3(p_ifstream_setbuf, &ifs, buffer, ARRAY_SIZE(buffer));
> +    ok(psb == NULL, "wrong return, expected NULL got %p\n", pfb);
Typo: pfb -> psb.

There are also some compilation warnings when compiling tests (i386).

Thanks,
Piotr



More information about the wine-devel mailing list