[PATCH 0/3] MR4803: dlls/gdiplus: Assorted path changes
Signed-off-by: David Kahurani k.kahurani(a)gmail.com -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4803
From: David Kahurani <k.kahurani(a)gmail.com> Serves to strictly allocate the requested size to a path unlike lengthen_path whose algorithm allocates more than strictly required Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/gdiplus/gdiplus.c | 18 ++++++++++++++++++ dlls/gdiplus/gdiplus_private.h | 1 + dlls/gdiplus/graphicspath.c | 19 ++----------------- dlls/gdiplus/metafile.c | 11 ++--------- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/dlls/gdiplus/gdiplus.c b/dlls/gdiplus/gdiplus.c index 32e287b0409..020a9de8acc 100644 --- a/dlls/gdiplus/gdiplus.c +++ b/dlls/gdiplus/gdiplus.c @@ -442,6 +442,24 @@ BOOL lengthen_path(GpPath *path, INT len) return TRUE; } +BOOL lengthen_path_strict(GpPath *path, INT len) +{ + /* Strictly allocate the requested memory size */ + /* Used only on fresh paths */ + path->pathdata.Points = calloc(len, sizeof(PointF)); + path->pathdata.Types = calloc(1, len); + + if (!path->pathdata.Points || !path->pathdata.Types) + { + free(path->pathdata.Points); + free(path->pathdata.Types); + free(path); + return FALSE; + } + + return TRUE; +} + void convert_32bppARGB_to_32bppPARGB(UINT width, UINT height, BYTE *dst_bits, INT dst_stride, const BYTE *src_bits, INT src_stride) { diff --git a/dlls/gdiplus/gdiplus_private.h b/dlls/gdiplus/gdiplus_private.h index 6f7e72124c2..0dae222ed12 100644 --- a/dlls/gdiplus/gdiplus_private.h +++ b/dlls/gdiplus/gdiplus_private.h @@ -130,6 +130,7 @@ extern void calc_curve_bezier_endp(REAL xend, REAL yend, REAL xadj, REAL yadj, extern void free_installed_fonts(void); extern BOOL lengthen_path(GpPath *path, INT len); +extern BOOL lengthen_path_strict(GpPath *path, INT len); extern DWORD write_region_data(const GpRegion *region, void *data); extern DWORD write_path_data(GpPath *path, void *data); diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c index 24c2888cfe8..7b85355773a 100644 --- a/dlls/gdiplus/graphicspath.c +++ b/dlls/gdiplus/graphicspath.c @@ -1184,14 +1184,7 @@ GpStatus WINGDIPAPI GdipClonePath(GpPath* path, GpPath **clone) **clone = *path; - (*clone)->pathdata.Points = malloc(path->datalen * sizeof(PointF)); - (*clone)->pathdata.Types = malloc(path->datalen); - if(!(*clone)->pathdata.Points || !(*clone)->pathdata.Types){ - free((*clone)->pathdata.Points); - free((*clone)->pathdata.Types); - free(*clone); - return OutOfMemory; - } + if (!lengthen_path_strict(*clone, path->datalen)) return OutOfMemory; memcpy((*clone)->pathdata.Points, path->pathdata.Points, path->datalen * sizeof(PointF)); @@ -1284,15 +1277,7 @@ GpStatus WINGDIPAPI GdipCreatePath2(GDIPCONST GpPointF* points, } } - (*path)->pathdata.Points = malloc(count * sizeof(PointF)); - (*path)->pathdata.Types = malloc(count); - - if(!(*path)->pathdata.Points || !(*path)->pathdata.Types){ - free((*path)->pathdata.Points); - free((*path)->pathdata.Types); - free(*path); - return OutOfMemory; - } + if (!lengthen_path_strict(*path, count)) return OutOfMemory; memcpy((*path)->pathdata.Points, points, count * sizeof(PointF)); memcpy((*path)->pathdata.Types, types, count); diff --git a/dlls/gdiplus/metafile.c b/dlls/gdiplus/metafile.c index 5c50d0d1d37..30e69ee4668 100644 --- a/dlls/gdiplus/metafile.c +++ b/dlls/gdiplus/metafile.c @@ -2039,16 +2039,9 @@ static GpStatus metafile_deserialize_path(const BYTE *record_data, UINT data_siz if (status != Ok) return status; - (*path)->pathdata.Count = data->PathPointCount; - (*path)->pathdata.Points = malloc(data->PathPointCount * sizeof(*(*path)->pathdata.Points)); - (*path)->pathdata.Types = malloc(data->PathPointCount * sizeof(*(*path)->pathdata.Types)); - (*path)->datalen = (*path)->pathdata.Count; + if (!lengthen_path_strict(*path, data->PathPointCount)) return OutOfMemory; - if (!(*path)->pathdata.Points || !(*path)->pathdata.Types) - { - GdipDeletePath(*path); - return OutOfMemory; - } + (*path)->datalen = (*path)->pathdata.Count; if (data->PathPointFlags & 0x4000) /* C */ { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4803
From: David Kahurani <k.kahurani(a)gmail.com> Empty the path, albeit temporarily before trying to lengthen the path because the data contained in it is at this point, unless an error happens, invalid. This avoids a situation where lengthening code, assuming the data in the path is valid proceeds to lengthen the path further. Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/gdiplus/graphicspath.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c index 7b85355773a..ea9f1ed9aed 100644 --- a/dlls/gdiplus/graphicspath.c +++ b/dlls/gdiplus/graphicspath.c @@ -1333,7 +1333,7 @@ GpStatus WINGDIPAPI GdipFlattenPath(GpPath *path, GpMatrix* matrix, REAL flatnes { path_list_node_t *list, *node; GpPointF pt; - INT i = 1; + INT i = 1, old_count; INT startidx = 0; GpStatus stat; @@ -1345,6 +1345,7 @@ GpStatus WINGDIPAPI GdipFlattenPath(GpPath *path, GpMatrix* matrix, REAL flatnes if(path->pathdata.Count == 0) return Ok; + old_count = path->pathdata.Count; stat = GdipTransformPath(path, matrix); if(stat != Ok) return stat; @@ -1407,6 +1408,7 @@ GpStatus WINGDIPAPI GdipFlattenPath(GpPath *path, GpMatrix* matrix, REAL flatnes /* store path data back */ i = path_list_count(list); + path->pathdata.Count = 0; if(!lengthen_path(path, i)) goto memout; path->pathdata.Count = i; @@ -1422,6 +1424,7 @@ GpStatus WINGDIPAPI GdipFlattenPath(GpPath *path, GpMatrix* matrix, REAL flatnes return Ok; memout: + path->pathdata.Count = old_count; free_path_list(list); return OutOfMemory; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4803
From: David Kahurani <k.kahurani(a)gmail.com> Empty the path, albeit temporarily before trying to lengthen the path because the data contained in it is at this point, unless an error happens, invalid. This avoids a situation where lengthening code, assuming the data in the path is valid proceeds to lengthen the path further. Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/gdiplus/graphicspath.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dlls/gdiplus/graphicspath.c b/dlls/gdiplus/graphicspath.c index ea9f1ed9aed..b40e84c07f3 100644 --- a/dlls/gdiplus/graphicspath.c +++ b/dlls/gdiplus/graphicspath.c @@ -2465,7 +2465,7 @@ GpStatus WINGDIPAPI GdipWidenPath(GpPath *path, GpPen *pen, GpMatrix *matrix, GpPath *flat_path=NULL; GpStatus status; path_list_node_t *points=NULL, *last_point=NULL; - int i, subpath_start=0, new_length; + int i, subpath_start=0, new_length, old_count; TRACE("(%p,%p,%s,%0.2f)\n", path, pen, debugstr_matrix(matrix), flatness); @@ -2475,6 +2475,8 @@ GpStatus WINGDIPAPI GdipWidenPath(GpPath *path, GpPen *pen, GpMatrix *matrix, if (path->pathdata.Count <= 1) return OutOfMemory; + old_count = path->pathdata.Count; + status = GdipClonePath(path, &flat_path); if (status == Ok) @@ -2550,8 +2552,12 @@ GpStatus WINGDIPAPI GdipWidenPath(GpPath *path, GpPen *pen, GpMatrix *matrix, new_length = path_list_count(points)-1; + path->pathdata.Count = 0; if (!lengthen_path(path, new_length)) + { + path->pathdata.Count = old_count; status = OutOfMemory; + } } if (status == Ok) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4803
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=141650 Your paranoid android. === debian11 (32 bit report) === gdiplus: metafile.c:3986: Test failed: Expected -00065536, got -16777216 metafile.c:3990: Test failed: Expected -00065536, got -16777216 === debian11b (64 bit WoW report) === gdiplus: metafile.c:3986: Test failed: Expected -00065536, got -16777216 metafile.c:3990: Test failed: Expected -00065536, got -16777216
Esme Povirk (@madewokherd) commented about dlls/gdiplus/gdiplus.c:
return TRUE; }
+BOOL lengthen_path_strict(GpPath *path, INT len) +{ + /* Strictly allocate the requested memory size */ + /* Used only on fresh paths */ + path->pathdata.Points = calloc(len, sizeof(PointF)); + path->pathdata.Types = calloc(1, len); + + if (!path->pathdata.Points || !path->pathdata.Types) + { + free(path->pathdata.Points); + free(path->pathdata.Types); + free(path);
If this can free the path as a side-effect, it should also create it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4803#note_57989
Esme Povirk (@madewokherd) commented about dlls/gdiplus/graphicspath.c:
/* store path data back */ i = path_list_count(list); + path->pathdata.Count = 0;
I think it would make more sense to pass in the number of additional points needed. Either that or construct an entirely new GpPathData and then free/replace the old one in case of error. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4803#note_57992
We should probably just have a function that replaces a path's contents with the contents of a point list. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4803#note_58004
participants (4)
-
David Kahurani -
David Kahurani (＠kahurani)
-
Esme Povirk (＠madewokherd)
-
Marvin