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

Re: [PATCH v15 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) 18 Jan '24

18 Jan '24

On Thu Jan 18 20:50:51 2024 +0000, Vijay Kiran Kamuju wrote: > Here it's about the test for GdipGetEffectParameterSize() with invalid > parameters - null effect and zero size. Hence passing a null effect. You should pass in NULL as a constant so it's easier to tell that's what you're doing. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58025

1 0

Re: [PATCH v15 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) 18 Jan '24

18 Jan '24

Esme Povirk (@madewokherd) commented about dlls/gdiplus/image.c: > + if (GdipGetEffectParameterSize(effect, &paramsize) != Ok) > + return InvalidParameter; > + > + switch (effect->type) > + { > + case BlurEffect: > + if (paramsize != size) > + return InvalidParameter; > + effect->p.blurparams = malloc(size); > + memcpy(effect->p.blurparams, params, size); > + break; > + case SharpenEffect: > + if (paramsize != size) > + return InvalidParameter; > + effect->p.sharpenparams = malloc(size); > + memcpy(effect->p.sharpenparams, params, size); I don't think it makes sense to duplicate this for each type. The member of the union may be different, but the code is equivalent. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58024

1 0

Re: [PATCH v15 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) 18 Jan '24

18 Jan '24

Esme Povirk (@madewokherd) commented about dlls/gdiplus/image.c: > { > ef = malloc(sizeof(CGpEffect)); > ef->type = ColorBalanceEffect; > + ef->p.clrbalanceparams = NULL; > } > else if (IsEqualGUID(&guid, &LevelsEffectGuid)) > { > ef = malloc(sizeof(CGpEffect)); > ef->type = LevelsEffect; > + ef->p.levelsparams = NULL; > } > else if (IsEqualGUID(&guid, &ColorCurveEffectGuid)) > { > ef = malloc(sizeof(CGpEffect)); > ef->type = ColorCurveEffect; > + ef->p.clrcurveparams = NULL; I don't think it makes sense to duplicate this assignment to NULL. I would just have a member of the union that is a BYTE* and use that when the type doesn't matter. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58023

1 0

Re: [PATCH v15 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Vijay Kiran Kamuju (＠infyquest) 18 Jan '24

18 Jan '24

On Thu Jan 18 20:41:13 2024 +0000, Esme Povirk wrote: > effect is supposed to be NULL here, so it doesn't make sense to pass it > in by name. Here it's about the test for GdipGetEffectParameterSize() with invalid parameters - null effect and zero size. Hence passing a null effect. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58022

1 0

[PATCH 0/5] MR576: vkd3d-common: Avoid (U)LONGs.
by Henri Verbeet (＠hverbeet) 18 Jan '24

18 Jan '24

-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/576

3 6

[PATCH 0/2] MR4872: concrt140: Add _Cancellation_beacon::_Confirm_cancel() implementation.
by Piotr Caban (＠piotr) 18 Jan '24

18 Jan '24

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4872

3 3

Re: [PATCH v15 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) 18 Jan '24

18 Jan '24

Esme Povirk (@madewokherd) commented about dlls/gdiplus/image.c: > + else if (IsEqualGUID(&guid, &ColorBalanceEffectGuid)) > + { > + ef = malloc(sizeof(CGpEffect)); > + ef->type = ColorBalanceEffect; > + } > + else if (IsEqualGUID(&guid, &LevelsEffectGuid)) > + { > + ef = malloc(sizeof(CGpEffect)); > + ef->type = LevelsEffect; > + } > + else if (IsEqualGUID(&guid, &ColorCurveEffectGuid)) > + { > + ef = malloc(sizeof(CGpEffect)); > + ef->type = ColorCurveEffect; > + } > + else This could be simplified by moving the `malloc` call to after the big if statement, and storing the type in a local variable. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58019

1 0

[PATCH 0/1] MR575: vkd3d-shader/d3dbc: Avoid DWORDs.
by Henri Verbeet (＠hverbeet) 18 Jan '24

18 Jan '24

-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/575

3 2

Re: [PATCH v15 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) 18 Jan '24

18 Jan '24

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); > + expect(InvalidParameter, stat); > + > stat = pGdipCreateEffect(noneffect, &effect); > expect(Win32Error, stat); > ok(effect == NULL, "Expected effect to be NULL\n"); > > - for(i=0; i < ARRAY_SIZE(effectlist); i++) > + size = 0; > + stat = pGdipGetEffectParameterSize(effect, &size); effect is supposed to be NULL here, so it doesn't make sense to pass it in by name. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58017

1 0

Re: [PATCH v15 0/4] MR4661: gdiplus: Implement Gdiplus effects
by Esme Povirk (＠madewokherd) 18 Jan '24

18 Jan '24

Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/image.c: > expect(InvalidParameter, stat); > > stat = pGdipCreateEffect(noneffect, &effect); > - todo_wine expect(Win32Error, stat); > + expect(Win32Error, stat); > + ok(effect == NULL, "Expected effect to be NULL\n"); effect should be initialized to non-NULL before calling GdipCreateEffect, in case it coincidentally started out as NULL. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4661#note_58016

1 0