June 2017 - Wine-devel - list.winehq.org

Re: [PATCH v2] api-ms-win-core-crt-l1-1-0: Add dll.
by Alexandre Julliard 22 Jun '17

22 Jun '17

Andrey Gusev <andrey.goosev(a)gmail.com> writes: > +@ cdecl -arch=i386 -ret64 _ftol2() MSVCRT__ftol > +@ cdecl -arch=i386 -ret64 _ftol2_sse() MSVCRT__ftol This can't work, the function isn't in this dll. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH] oleaut32: check return code of Currency2Integer (Coverity)
by Dmitry Timoshkov 22 Jun '17

22 Jun '17

Marcus Meissner <marcus(a)jet.franken.de> wrote: > HRESULT WINAPI VarI1FromCy(CY cyIn, signed char* pcOut) > { > LONG i = I1_MAX + 1; > + HRESUTL hr; > > - VarI4FromCy(cyIn, &i); > - return _VarI1FromI4(i, pcOut); > + hr = VarI4FromCy(cyIn, &i); > + > + if (SUCCEEDED(hr)) > + hr = _VarI1FromI4(i, pcOut); > + return hr; > } I guess that this change makes initialization of 'i' redundant. Here and in remaining cases. Probably needs a test case though to make sure which behaviour is more correct. -- Dmitry.

1 0

Re: rsaenh: Avoid double 'va_end(args)' in case of a premature loop termination.
by Sebastian Lackner 22 Jun '17

22 Jun '17

On 21.06.2017 10:24, Dmitry Timoshkov wrote: > Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> > --- > dlls/rsaenh/mpi.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/dlls/rsaenh/mpi.c b/dlls/rsaenh/mpi.c > index 03e31023e6..dcf6ddf112 100644 > --- a/dlls/rsaenh/mpi.c > +++ b/dlls/rsaenh/mpi.c > @@ -2365,10 +2365,7 @@ int mp_init_multi(mp_int *mp, ...) > succeeded in init-ing, then return error. > */ > va_list clean_args; > - > - /* end the current list */ > - va_end(args); > - > + > /* now start cleaning up */ > cur_arg = mp; > va_start(clean_args, mp); > @@ -2381,6 +2378,7 @@ int mp_init_multi(mp_int *mp, ...) > break; > } > n++; > + va_end(args); Doesn't this change make it even worse? Whats the purpose of calling va_end() when the enumeration isn't finished yet? > cur_arg = va_arg(args, mp_int*); > } > va_end(args); >

2 3

Re: [11/17] windowscodecs: Simplify a bit comparison of two PROPVARIANTs.
by Vincent Povirk 22 Jun '17

22 Jun '17

I'm not sure the point of changing this when we're just going to replace it with PropVariantCompareEx.

2 1

Re: [13/17] windowscodecs: Use PropVariantCompareEx to compare metadata items.
by Vincent Povirk 22 Jun '17

22 Jun '17

This breaks the windowscodecs/metadata test here. fixme:propsys:PropVariantCompareEx vartype 30 not handled metadata.c:382: Test failed: GetValue failed, hr=88982f40 fixme:propsys:PropVariantCompareEx vartype 30 not handled metadata.c:388: Test failed: GetValue failed, hr=88982f40 metadata.c:389: Test failed: unexpected vt: 30 wine: Unhandled page fault on read access to 0x00000000 at address 0x7ed13c05 (thread 0009), starting debugger...

2 1

Re: [v4 1/4] d3dx9: Remove redundant parameter size check in set_constants().
by Matteo Bruni 21 Jun '17

21 Jun '17

2017-06-19 11:24 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > v4: > - added assert() for parameter size. > --- > dlls/d3dx9_36/preshader.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c > index 2785ca3..76d6de9 100644 > --- a/dlls/d3dx9_36/preshader.c > +++ b/dlls/d3dx9_36/preshader.c > @@ -1178,11 +1178,6 @@ static void set_constants(struct d3dx_regstore *rs, struct d3dx_const_tab *const > param_offset = i + j * info.major; > else > param_offset = i * info.minor + j; > - if (param_offset * sizeof(unsigned int) >= param->bytes) > - { > - WARN("Parameter data is too short, name %s, component %u.\n", debugstr_a(param->name), i); > - break; > - } > out[offset] = data[param_offset]; > } > } > @@ -1303,6 +1298,11 @@ static HRESULT merge_const_set_entries(struct d3dx_const_tab *const_tab, > memmove(&const_tab->const_set[index + 1], &const_tab->const_set[i], > sizeof(*const_tab->const_set) * (const_tab->const_set_count - i)); > const_tab->const_set_count -= i - index - 1; > + assert(param->bytes >= table_info[first_const->table].component_size > + * first_const->element_count > + * (first_const->direct_copy ? get_reg_components(first_const->table) > + * first_const->register_count > + : first_const->param->rows * first_const->param->columns)); I don't think an assert() is correct here. You are comparing parameter size, which eventually depends on the effect data, with the constant size, which comes from the constant table and thus again the effect data. In general we shouldn't assert on invalid input data. Also by putting the assert() in merge_const_set_entries() you're not checking the non-array, non-struct constants. Am I missing something? Is there something else that would trigger if e.g. a 4x4 matrix constants gets matched with a 3x4 parameter?

2 4

Re: [PATCH 1/2] quartz: Restore a comment explaining mediatime.
by Matteo Bruni 21 Jun '17

21 Jun '17

2017-06-19 21:15 GMT+02:00 Zebediah Figura <z.figura12(a)gmail.com>: > This comment was removed by 0a6f11c8, possibly by accident. > The usage of the term "media time" here is confusing, both > because it does not describe time (or any meaningful unit) > and also because it conflicts with the normal definition of > "media time", viz. length in number of samples. > > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/quartz/quartz_private.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/dlls/quartz/quartz_private.h b/dlls/quartz/quartz_private.h > index 7cad3a75640..a9855e95acd 100644 > --- a/dlls/quartz/quartz_private.h > +++ b/dlls/quartz/quartz_private.h > @@ -33,6 +33,7 @@ > #include "wine/strmbase.h" > #include "wine/list.h" > > +/* see IAsyncReader::Request on MSDN for the explanation of this */ > #define MEDIATIME_FROM_BYTES(x) ((LONGLONG)(x) * 10000000) > #define SEC_FROM_MEDIATIME(time) ((time) / 10000000) > #define BYTES_FROM_MEDIATIME(time) SEC_FROM_MEDIATIME(time) Is it reasonable to put some short explanation right here (i.e. removing the middle man) instead?

2 1

Re: [v4 4/4] d3dx9: Move transpose 'if' outside of the inner loop in set_constants().
by Matteo Bruni 21 Jun '17

21 Jun '17

2017-06-19 11:24 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > v4: > - updated to handle incomplete row of transposed matrix accordingly. > --- > dlls/d3dx9_36/preshader.c | 26 ++++++++++---------------- > 1 file changed, 10 insertions(+), 16 deletions(-) > > diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c > index 9a2ecfa..39bd483 100644 > --- a/dlls/d3dx9_36/preshader.c > +++ b/dlls/d3dx9_36/preshader.c > @@ -1138,7 +1138,6 @@ static void set_constants(struct d3dx_regstore *rs, struct d3dx_const_tab *const > unsigned int table = const_set->table; > struct d3dx_parameter *param = const_set->param; > unsigned int element, i, j, start_offset; > - unsigned int param_offset; > struct const_upload_info info; > unsigned int *data = param->data; > enum pres_value_type param_type; > @@ -1178,24 +1177,19 @@ static void set_constants(struct d3dx_regstore *rs, struct d3dx_const_tab *const > > /* Store reshaped but (possibly) not converted yet data temporarily in the same constants buffer. > * All the supported types of parameters and table values have the same size. */ > - for (i = 0; i < info.major_count; ++i) > - { > - for (j = 0; j < info.minor; ++j) > - { > - if (info.transpose) > - param_offset = i + j * info.major; > - else > - param_offset = i * info.minor + j; > - out[i * info.major_stride + j] = data[param_offset]; > - } > - } > if (info.transpose) > { > + for (i = 0; i < info.major_count; ++i) > + for (j = 0; j < info.minor; ++j) > + out[i * info.major_stride + j] = data[i + j * info.major]; > + > for (j = 0; j < info.minor_remainder; ++j) > - { > - param_offset = i + j * info.major; > - out[i * info.major_stride + j] = data[param_offset]; > - } > + out[i * info.major_stride + j] = data[i + j * info.major]; } Accidental backspace? ;)

1 0

Re: [PATCH v2 2/4] winemp3.acm: Disallow operations encoding to MPEG.
by Andrew Eikum 21 Jun '17

21 Jun '17

I'd like this more with tests, showing what the application was doing and how Wine failed its expectations but now meets them. Andrew On Fri, Jun 09, 2017 at 06:08:28PM -0500, Zebediah Figura wrote: > Since the winemp3 driver is currently decode-only, and most Windows > versions do not ship by default with a driver capable of encoding > to mpeg3, we should report the driver's capabilities accordingly. > > v2: remove failing tests > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/winemp3.acm/mpegl3.c | 80 ++++++++++------------------------------------- > 1 file changed, 17 insertions(+), 63 deletions(-) > > diff --git a/dlls/winemp3.acm/mpegl3.c b/dlls/winemp3.acm/mpegl3.c > index cc6008da59e..c5d71bba524 100644 > --- a/dlls/winemp3.acm/mpegl3.c > +++ b/dlls/winemp3.acm/mpegl3.c > @@ -258,10 +258,13 @@ static LRESULT MPEG3_StreamOpen(PACMDRVSTREAMINSTANCE adsi) > mpg123_param(aad->mh, MPG123_ADD_FLAGS, MPG123_IGNORE_INFOFRAME, 0); > #endif > } > - /* no encoding yet > else if (adsi->pwfxSrc->wFormatTag == WAVE_FORMAT_PCM && > - adsi->pwfxDst->wFormatTag == WAVE_FORMAT_MPEGLAYER3) > - */ > + (adsi->pwfxDst->wFormatTag == WAVE_FORMAT_MPEGLAYER3 || > + adsi->pwfxDst->wFormatTag == WAVE_FORMAT_MPEG)) > + { > + WARN("Encoding to MPEG is not supported\n"); > + goto theEnd; > + } > else goto theEnd; > MPEG3_Reset(adsi, aad); > > @@ -736,54 +739,19 @@ static LRESULT MPEG3_FormatTagDetails(PACMFORMATTAGDETAILSW aftd, DWORD dwQuery) > case 1: > aftd->dwFormatTag = WAVE_FORMAT_MPEGLAYER3; > aftd->cbFormatSize = sizeof(MPEGLAYER3WAVEFORMAT); > - aftd->cStandardFormats = NUM_MPEG3_FORMATS; > + aftd->cStandardFormats = 0; > lstrcpyW(aftd->szFormatTag, szMpeg3); > break; > case 2: > aftd->dwFormatTag = WAVE_FORMAT_MPEG; > aftd->cbFormatSize = sizeof(MPEG1WAVEFORMAT); > - aftd->cStandardFormats = NUM_MPEG3_FORMATS; > + aftd->cStandardFormats = 0; > lstrcpyW(aftd->szFormatTag, szMpeg); > break; > } > return MMSYSERR_NOERROR; > } > > -static void fill_in_mp3(unsigned cbwfx, WAVEFORMATEX* wfx, unsigned bit_rate) > -{ > - MPEGLAYER3WAVEFORMAT* mp3wfx = (MPEGLAYER3WAVEFORMAT*)wfx; > - > - wfx->nAvgBytesPerSec = bit_rate / 8; > - if (cbwfx >= sizeof(WAVEFORMATEX)) > - wfx->cbSize = sizeof(MPEGLAYER3WAVEFORMAT) - sizeof(WAVEFORMATEX); > - if (cbwfx >= sizeof(MPEGLAYER3WAVEFORMAT)) > - { > - mp3wfx->wID = MPEGLAYER3_ID_MPEG; > - mp3wfx->fdwFlags = MPEGLAYER3_FLAG_PADDING_OFF; > - mp3wfx->nBlockSize = (bit_rate * 144) / wfx->nSamplesPerSec; > - mp3wfx->nFramesPerBlock = 1; > - mp3wfx->nCodecDelay = 0x0571; > - } > -} > - > -static void fill_in_mpeg(unsigned cbwfx, WAVEFORMATEX* wfx, unsigned bit_rate) > -{ > - MPEG1WAVEFORMAT* mp3wfx = (MPEG1WAVEFORMAT*)wfx; > - > - wfx->nAvgBytesPerSec = bit_rate / 8; > - if (cbwfx >= sizeof(WAVEFORMATEX)) > - wfx->cbSize = sizeof(MPEG1WAVEFORMAT) - sizeof(WAVEFORMATEX); > - if (cbwfx >= sizeof(MPEG1WAVEFORMAT)) > - { > - mp3wfx->fwHeadLayer = ACM_MPEG_LAYER3; > - mp3wfx->dwHeadBitrate = wfx->nAvgBytesPerSec * 8; > - mp3wfx->fwHeadMode = ACM_MPEG_JOINTSTEREO; > - mp3wfx->fwHeadModeExt = 0xf; > - mp3wfx->wHeadEmphasis = 1; > - mp3wfx->fwHeadFlags = ACM_MPEG_ID_MPEG1; > - } > -} > - > /*********************************************************************** > * MPEG3_FormatDetails > * > @@ -814,16 +782,8 @@ static LRESULT MPEG3_FormatDetails(PACMFORMATDETAILSW afd, DWORD dwQuery) > break; > case WAVE_FORMAT_MPEGLAYER3: > case WAVE_FORMAT_MPEG: > - if (afd->dwFormatIndex >= NUM_MPEG3_FORMATS) return ACMERR_NOTPOSSIBLE; > - afd->pwfx->nChannels = MPEG3_Formats[afd->dwFormatIndex].nChannels; > - afd->pwfx->nSamplesPerSec = MPEG3_Formats[afd->dwFormatIndex].rate; > - afd->pwfx->wBitsPerSample = MPEG3_Formats[afd->dwFormatIndex].nBits; > - afd->pwfx->nBlockAlign = 1; > - if (afd->dwFormatTag == WAVE_FORMAT_MPEGLAYER3) > - fill_in_mp3(afd->cbwfx, afd->pwfx, 192000); > - else > - fill_in_mpeg(afd->cbwfx, afd->pwfx, 192000); > - break; > + WARN("Encoding to MPEG is not supported\n"); > + return ACMERR_NOTPOSSIBLE; > default: > WARN("Unsupported tag %08x\n", afd->dwFormatTag); > return MMSYSERR_INVALPARAM; > @@ -856,18 +816,15 @@ static LRESULT MPEG3_FormatSuggest(PACMDRVFORMATSUGGEST adfs) > adfs->pwfxDst->nChannels = adfs->pwfxSrc->nChannels; > if (!(adfs->fdwSuggest & ACM_FORMATSUGGESTF_NSAMPLESPERSEC)) > adfs->pwfxDst->nSamplesPerSec = adfs->pwfxSrc->nSamplesPerSec; > - > if (!(adfs->fdwSuggest & ACM_FORMATSUGGESTF_WBITSPERSAMPLE)) > - { > - if (adfs->pwfxSrc->wFormatTag == WAVE_FORMAT_PCM) > - adfs->pwfxDst->wBitsPerSample = 4; > - else > - adfs->pwfxDst->wBitsPerSample = 16; > - } > + adfs->pwfxDst->wBitsPerSample = 16; > if (!(adfs->fdwSuggest & ACM_FORMATSUGGESTF_WFORMATTAG)) > { > if (adfs->pwfxSrc->wFormatTag == WAVE_FORMAT_PCM) > - adfs->pwfxDst->wFormatTag = WAVE_FORMAT_MPEGLAYER3; > + { > + WARN("Encoding to MPEG is not supported\n"); > + return ACMERR_NOTPOSSIBLE; > + } > else > adfs->pwfxDst->wFormatTag = WAVE_FORMAT_PCM; > } > @@ -883,12 +840,9 @@ static LRESULT MPEG3_FormatSuggest(PACMDRVFORMATSUGGEST adfs) > adfs->pwfxDst->nAvgBytesPerSec = adfs->pwfxDst->nSamplesPerSec * adfs->pwfxDst->nBlockAlign; > break; > case WAVE_FORMAT_MPEG: > - adfs->pwfxDst->nBlockAlign = 1; > - fill_in_mpeg(adfs->cbwfxDst, adfs->pwfxDst, 192000); > - break; > case WAVE_FORMAT_MPEGLAYER3: > - adfs->pwfxDst->nBlockAlign = 1; > - fill_in_mp3(adfs->cbwfxDst, adfs->pwfxDst, 192000); > + WARN("Encoding to MPEG is not supported\n"); > + return ACMERR_NOTPOSSIBLE; > break; > default: > FIXME("\n"); > -- > 2.13.0 > > >

2 4

Re: user.exe16: Fix memory leaks in cases when HeapReAlloc fails.
by Alexandre Julliard 21 Jun '17

21 Jun '17

Dmitry Timoshkov <dmitry(a)baikal.ru> writes: > @@ -3286,7 +3289,8 @@ DWORD WINAPI FormatMessage16( > } > talloced = strlen(target)+1; > if (nSize && talloced<nSize) { > - target = HeapReAlloc(GetProcessHeap(),HEAP_ZERO_MEMORY,target,nSize); > + LPSTR new_target = HeapReAlloc(GetProcessHeap(),HEAP_ZERO_MEMORY,target,nSize); > + if (new_target) target = new_target; You could probably remove it entirely. -- Alexandre Julliard julliard(a)winehq.org

1 0