January 2024 - Wine-GitLab - list.winehq.org

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Vijay Kiran Kamuju (＠infyquest) Jan. 22, 2024

Jan. 22, 2024

On Mon Jan 22 18:25:33 2024 +0000, Esme Povirk wrote: > This doesn't look right to me. Assigning to desc overwrites the value > passed into the function. I will add a check if desc is already allocated or not, if yes then use realloc. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58440

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Vijay Kiran Kamuju (＠infyquest) Jan. 22, 2024

Jan. 22, 2024

On Mon Jan 22 18:34:33 2024 +0000, Esme Povirk wrote: > I think all the cases except RedEyeCorrectionEffect can be combined into > one case. I don't think the `default:` case should be reachable. They can all be combined to one, if we don't use a union and use 'void *' as member of CGpEffect to store the parameter. I am a bit confused on how to merge the cases if we use a union. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58439

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) Jan. 22, 2024

Jan. 22, 2024

Esme Povirk (@madewokherd) commented about dlls/gdiplus/image.c: > + { > + if (paramsize != size) > + return InvalidParameter; > + } > + > + switch (effect->type) > + { > + case BlurEffect: > + alloc_copy(params, effect->p.blurparams, size); > + break; > + case SharpenEffect: > + alloc_copy(params, effect->p.sharpenparams, size); > + break; > + case TintEffect: > + alloc_copy(params, effect->p.tintparams, size); > + break; I think all the cases except RedEyeCorrectionEffect can be combined into one case. I don't think the `default:` case should be reachable. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58438

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) Jan. 22, 2024

Jan. 22, 2024

Esme Povirk (@madewokherd) commented about dlls/gdiplus/image.c: > + > + if (effect->type == RedEyeCorrectionEffect) > + { > + if ((paramsize-size > 0) || (((size-paramsize)%sizeof(RECT)) != 0)) > + return InvalidParameter; > + } > + else > + { > + if (paramsize != size) > + return InvalidParameter; > + } > + > + switch (effect->type) > + { > + case BlurEffect: > + alloc_copy(params, effect->p.blurparams, size); I think this will leak memory if parameters were already set. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58437

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) Jan. 22, 2024

Jan. 22, 2024

Esme Povirk (@madewokherd) commented about dlls/gdiplus/image.c: > > TRACE("(%p,%p,%u)\n", effect, params, size); > > - if(!(calls++)) > - FIXME("not implemented\n"); > + if (!effect || !params || !size) > + return InvalidParameter; > > - return NotImplemented; > + if (GdipGetEffectParameterSize(effect, &paramsize) != Ok) > + return InvalidParameter; > + > + if (effect->type == RedEyeCorrectionEffect) > + { > + if ((paramsize-size > 0) || (((size-paramsize)%sizeof(RECT)) != 0)) > + return InvalidParameter; I don't think we can use `paramsize` in this case because it will depend on whether parameters were already set. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58436

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) Jan. 22, 2024

Jan. 22, 2024

Esme Povirk (@madewokherd) commented about dlls/gdiplus/image.c: > status = InvalidParameter; > break; > } > - > *size = sz; > + > return status; > } > > +void alloc_copy(const void *src, void *desc, UINT size) > +{ > + desc = malloc(size); > + memcpy(desc, src, size); > +} This doesn't look right to me. Assigning to desc overwrites the value passed into the function. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58435

1 0

Re: [PATCH v18 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) Jan. 22, 2024

Jan. 22, 2024

Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/image.c: > stat = pGdipCreateEffect(BlurEffectGuid, NULL); > expect(InvalidParameter, stat); > > + stat = pGdipGetEffectParameterSize(NULL, NULL); > + expect(InvalidParameter, stat); > + > + stat = pGdipGetEffectParameterSize(effect, NULL); This uses effect uninitialized. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58434

1 0

[PATCH v4 0/2] MR4830: dinput: implement DIPROP_AUTOCENTER
by Tyson Whitehead (＠twhitehead) Jan. 22, 2024

Jan. 22, 2024

Autocenter is a default effect playing on the device following power up or reset. It is disabled by stopping all effects. On at least some devices (all?) it is a spring effect playing in slot 1. Capturing Windows USB packets reveals it dinput acquire does (1) a reset (this enabled autocenter) and, if autocenter is disabled, (2) a stop all effects (this disabled autocenter). This logic works regardless of whether autocenter is a spring effect playing in slot 1 or not. It does mean autocenter can only be set when the device is not acquired. Testing on Windows reveals setting autocenter properties while acquired returns DIERR_ACQUIRED even if the device is exclusively acquired, so this is consistent. -- v4: dinput: Update tests for DIPROP_AUTOCENTER dinput: Implement DIPROP_AUTOCENTER https://gitlab.winehq.org/wine/wine/-/merge_requests/4830

4 11

[PATCH 0/1] MR4910: setupapi: Don't clobber the original filename if .inf is found.
by Rémi Bernon Jan. 22, 2024

Jan. 22, 2024

Fixes some test failures with https://gitlab.winehq.org/wine/wine/-/merge_requests/4904, where when the driver is found again for a new device (which has a different VID/PID from the first created device and thus, is not cached yet), the original filename from the .pnf file has been overwritten and the driver modules are incorrectly looked up for relative to the .inf in the `c:\windows\inf` folder. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4910

1 1

[PATCH v2 0/4] MR4906: mf,mfplat: Clang warning fixes.
by Jacek Caban (＠jacek) Jan. 22, 2024

Jan. 22, 2024

-- v2: mfplat: Introduce media_type_get_uint32 helper. https://gitlab.winehq.org/wine/wine/-/merge_requests/4906

2 5