July 2024 - Wine-commits - list.winehq.org

Brendan McGrath : winegstreamer: Use process affinity to calculate thread_count.
by Alexandre Julliard 03 Jul '24

03 Jul '24

Module: wine Branch: master Commit: d4e95ede4f44a5c06278a27df929e8d9f6ce9263 URL: https://gitlab.winehq.org/wine/wine/-/commit/d4e95ede4f44a5c06278a27df929e8… Author: Brendan McGrath <bmcgrath(a)codeweavers.com> Date: Wed Jun 26 11:35:32 2024 +1000 winegstreamer: Use process affinity to calculate thread_count. --- dlls/winegstreamer/unixlib.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index 175ab92ecdc..fe432070cd8 100644 --- a/dlls/winegstreamer/unixlib.c +++ b/dlls/winegstreamer/unixlib.c @@ -47,6 +47,8 @@ GST_DEBUG_CATEGORY(wine); +static UINT thread_count; + GstStreamType stream_type_from_caps(GstCaps *caps) { const gchar *media_type; @@ -244,6 +246,17 @@ bool push_event(GstPad *pad, GstEvent *event) return true; } +static ULONG popcount(ULONG val) +{ +#if HAVE___BUILTIN_POPCOUNT + return __builtin_popcount(val); +#else + val -= val >> 1 & 0x55555555; + val = (val & 0x33333333) + (val >> 2 & 0x33333333); + return ((val + (val >> 4)) & 0x0f0f0f0f) * 0x01010101 >> 24; +#endif +} + NTSTATUS wg_init_gstreamer(void *arg) { struct wg_init_gstreamer_params *params = arg; @@ -253,6 +266,7 @@ NTSTATUS wg_init_gstreamer(void *arg) int argc = ARRAY_SIZE(args) - 1; char **argv = args; GError *err; + DWORD_PTR process_mask; if (params->trace_on) setenv("GST_DEBUG", "WINE:9,4", FALSE); @@ -276,6 +290,12 @@ NTSTATUS wg_init_gstreamer(void *arg) return STATUS_UNSUCCESSFUL; } + if (!NtQueryInformationProcess(GetCurrentProcess(), + ProcessAffinityMask, &process_mask, sizeof(process_mask), NULL)) + thread_count = popcount(process_mask); + else + thread_count = 0; + GST_DEBUG_CATEGORY_INIT(wine, "WINE", GST_DEBUG_FG_RED, "Wine GStreamer support"); GST_INFO("GStreamer library version %s; wine built with %d.%d.%d.",

1 0

Brendan McGrath : winegstreamer: Use thread_count to determine 'max-threads' value.
by Alexandre Julliard 03 Jul '24

03 Jul '24

Module: wine Branch: master Commit: ff5a198a8b35d19f772086ca70beee1c34b9a93c URL: https://gitlab.winehq.org/wine/wine/-/commit/ff5a198a8b35d19f772086ca70beee… Author: Brendan McGrath <bmcgrath(a)codeweavers.com> Date: Wed Jun 26 11:41:11 2024 +1000 winegstreamer: Use thread_count to determine 'max-threads' value. GStreamer uses _SC_NPROCESSORS_CONF to determine 'max-threads'. On the Steam Deck, this is configured to be 16 (which is double its number of logical cores). _SC_NPROCESSORS_CONF also disregards a process's CPU affinity, thus it can create more threads than is useful, which ultimately wastes memory resources. Using thread_count to set 'max-threads' addresses both these problems. --- dlls/winegstreamer/unix_private.h | 1 + dlls/winegstreamer/unixlib.c | 34 ++++++++++++++++++++++++++++++++++ dlls/winegstreamer/wg_parser.c | 7 +++++++ dlls/winegstreamer/wg_transform.c | 2 ++ 4 files changed, 44 insertions(+) diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 0c285457d78..b2cc036c914 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -46,6 +46,7 @@ extern bool link_src_to_sink(GstPad *src_pad, GstPad *sink_pad); extern bool link_src_to_element(GstPad *src_pad, GstElement *element); extern bool link_element_to_sink(GstElement *element, GstPad *sink_pad); extern bool push_event(GstPad *pad, GstEvent *event); +extern void set_max_threads(GstElement *element); /* wg_format.c */ diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index fe432070cd8..3558cf45423 100644 --- a/dlls/winegstreamer/unixlib.c +++ b/dlls/winegstreamer/unixlib.c @@ -302,3 +302,37 @@ NTSTATUS wg_init_gstreamer(void *arg) gst_version_string(), GST_VERSION_MAJOR, GST_VERSION_MINOR, GST_VERSION_MICRO); return STATUS_SUCCESS; } + +static bool element_has_property(const GstElement *element, const gchar *property) +{ + return !!g_object_class_find_property(G_OBJECT_CLASS(GST_ELEMENT_GET_CLASS(element)), property); +} + +void set_max_threads(GstElement *element) +{ + const char *shortname = NULL; + GstElementFactory *factory = gst_element_get_factory(element); + + if (factory) + shortname = gst_plugin_feature_get_name(GST_PLUGIN_FEATURE(factory)); + + /* By default, GStreamer will use the result of sysconf(_SC_NPROCESSORS_CONF) to determine the number + * of decoder threads to be used by libva. This has two issues: + * 1. It can return an inaccurate result (for example, on the Steam Deck this returns 16); and + * 2. It disregards process affinity + * + * Both of these scenarios result in more threads being allocated than logical cores made available, meaning + * they provide little (or possibly detrimental) performance benefit and for 4K video can occupy 32MB + * of RAM each (w * h * bpp). + * + * So we will instead explictly set 'max-threads' to the minimum of thread_count (process affinity at time of + * initialization) or 16. + */ + + if (shortname && strstr(shortname, "avdec_") && element_has_property(element, "max-threads")) + { + gint32 max_threads = MIN(thread_count, 16); + GST_DEBUG("%s found, setting max-threads to %d.", shortname, max_threads); + g_object_set(element, "max-threads", max_threads, NULL); + } +} diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 8690e6baf81..710cfe6a0a5 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -567,6 +567,12 @@ static void no_more_pads_cb(GstElement *element, gpointer user) pthread_cond_signal(&parser->init_cond); } +static void deep_element_added_cb(GstBin *self, GstBin *sub_bin, GstElement *element, gpointer user) +{ + if (element) + set_max_threads(element); +} + static gboolean sink_event_cb(GstPad *pad, GstObject *parent, GstEvent *event) { struct wg_parser_stream *stream = gst_pad_get_element_private(pad); @@ -1797,6 +1803,7 @@ static BOOL decodebin_parser_init_gst(struct wg_parser *parser) g_signal_connect(element, "autoplug-continue", G_CALLBACK(autoplug_continue_cb), parser); g_signal_connect(element, "autoplug-select", G_CALLBACK(autoplug_select_cb), parser); g_signal_connect(element, "no-more-pads", G_CALLBACK(no_more_pads_cb), parser); + g_signal_connect(element, "deep-element-added", G_CALLBACK(deep_element_added_cb), parser); pthread_mutex_lock(&parser->mutex); parser->no_more_pads = false; diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index cd6bb9c8f52..76dbd347064 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -555,6 +555,8 @@ NTSTATUS wg_transform_create(void *args) if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, parsed_caps, sink_caps)) || !append_element(transform->container, element, &first, &last)) goto out; + + set_max_threads(element); } if (g_str_has_prefix(output_mime, "audio/"))

1 0

Brendan McGrath : winegstreamer: Set 'max_threads' to 4 for 32-bit processors.
by Alexandre Julliard 03 Jul '24

03 Jul '24

Module: wine Branch: master Commit: 94fd0561b740dbb94608a8c4b3e1283e345e8c7a URL: https://gitlab.winehq.org/wine/wine/-/commit/94fd0561b740dbb94608a8c4b3e128… Author: Brendan McGrath <bmcgrath(a)codeweavers.com> Date: Wed Jun 26 12:22:24 2024 +1000 winegstreamer: Set 'max_threads' to 4 for 32-bit processors. The avdec_h264 element can use 32MB per thread when working with 4K video. With 16 threads, this is 512MB, which is a quarter of the RAM available to a 32-bit application. Setting 'max_threads' to 4 can save 384MB. --- dlls/winegstreamer/unixlib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index 3558cf45423..729008e1f46 100644 --- a/dlls/winegstreamer/unixlib.c +++ b/dlls/winegstreamer/unixlib.c @@ -326,12 +326,12 @@ void set_max_threads(GstElement *element) * of RAM each (w * h * bpp). * * So we will instead explictly set 'max-threads' to the minimum of thread_count (process affinity at time of - * initialization) or 16. + * initialization) or 16 (4 for 32-bit processors). */ if (shortname && strstr(shortname, "avdec_") && element_has_property(element, "max-threads")) { - gint32 max_threads = MIN(thread_count, 16); + gint32 max_threads = MIN(thread_count, sizeof(void *) == 4 ? 4 : 16); GST_DEBUG("%s found, setting max-threads to %d.", shortname, max_threads); g_object_set(element, "max-threads", max_threads, NULL); }

1 0

Rémi Bernon : winegstreamer: Normalize video processor and color converter apertures.
by Alexandre Julliard 03 Jul '24

03 Jul '24

Module: wine Branch: master Commit: c02a06fde1567d752e2c887d30845dba9d3ad8a0 URL: https://gitlab.winehq.org/wine/wine/-/commit/c02a06fde1567d752e2c887d30845d… Author: Rémi Bernon <rbernon(a)codeweavers.com> Date: Tue Jul 2 21:29:58 2024 +0200 winegstreamer: Normalize video processor and color converter apertures. --- dlls/winegstreamer/Makefile.in | 2 +- dlls/winegstreamer/color_convert.c | 58 +++++++++++++++++++++++++++++++++++- dlls/winegstreamer/gst_private.h | 23 ++++++++++++++ dlls/winegstreamer/video_processor.c | 22 ++++++++++++++ 4 files changed, 103 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/Makefile.in b/dlls/winegstreamer/Makefile.in index 447c2c74cf3..63ca3f61fdf 100644 --- a/dlls/winegstreamer/Makefile.in +++ b/dlls/winegstreamer/Makefile.in @@ -1,7 +1,7 @@ MODULE = winegstreamer.dll UNIXLIB = winegstreamer.so IMPORTLIB = winegstreamer -IMPORTS = strmbase ole32 oleaut32 msdmo msvcrt +IMPORTS = strmbase ole32 oleaut32 msdmo msvcrt user32 DELAYIMPORTS = mfplat mf UNIX_CFLAGS = $(GSTREAMER_CFLAGS) UNIX_LIBS = $(GSTREAMER_LIBS) $(PTHREAD_LIBS) diff --git a/dlls/winegstreamer/color_convert.c b/dlls/winegstreamer/color_convert.c index 21d2aa67e64..e466a9289b1 100644 --- a/dlls/winegstreamer/color_convert.c +++ b/dlls/winegstreamer/color_convert.c @@ -95,9 +95,59 @@ static inline struct color_convert *impl_from_IUnknown(IUnknown *iface) return CONTAINING_RECORD(iface, struct color_convert, IUnknown_inner); } +static void update_video_aperture(MFVideoInfo *input_info, MFVideoInfo *output_info) +{ + static const MFVideoArea empty_area = {0}; + + /* Tests show that the color converter ignores aperture entirely, probably a side + * effect of an internal conversion to VIDEOINFOHEADER2, as the component is also + * exposing a IMediaObject interface, and designed for dshow. + */ + + input_info->GeometricAperture = empty_area; + input_info->MinimumDisplayAperture = empty_area; + input_info->PanScanAperture = empty_area; + + output_info->GeometricAperture = empty_area; + output_info->MinimumDisplayAperture = empty_area; + output_info->PanScanAperture = empty_area; +} + +static HRESULT normalize_media_types(IMFMediaType **input_type, IMFMediaType **output_type) +{ + MFVIDEOFORMAT *input_format, *output_format; + UINT32 size; + HRESULT hr; + + if (FAILED(hr = MFCreateMFVideoFormatFromMFMediaType(*input_type, &input_format, &size))) + return hr; + if (FAILED(hr = MFCreateMFVideoFormatFromMFMediaType(*output_type, &output_format, &size))) + { + CoTaskMemFree(input_format); + return hr; + } + + update_video_aperture(&input_format->videoInfo, &output_format->videoInfo); + + if (FAILED(hr = MFCreateVideoMediaType(input_format, (IMFVideoMediaType **)input_type))) + goto done; + if (FAILED(hr = MFCreateVideoMediaType(output_format, (IMFVideoMediaType **)output_type))) + { + IMFMediaType_Release(*input_type); + *input_type = NULL; + } + +done: + CoTaskMemFree(input_format); + CoTaskMemFree(output_format); + return hr; +} + static HRESULT try_create_wg_transform(struct color_convert *impl) { + IMFMediaType *input_type = impl->input_type, *output_type = impl->output_type; struct wg_transform_attrs attrs = {0}; + HRESULT hr; if (impl->wg_transform) { @@ -105,7 +155,13 @@ static HRESULT try_create_wg_transform(struct color_convert *impl) impl->wg_transform = 0; } - return wg_transform_create_mf(impl->input_type, impl->output_type, &attrs, &impl->wg_transform); + if (FAILED(hr = normalize_media_types(&input_type, &output_type))) + return hr; + hr = wg_transform_create_mf(input_type, output_type, &attrs, &impl->wg_transform); + IMFMediaType_Release(output_type); + IMFMediaType_Release(input_type); + + return hr; } static HRESULT WINAPI unknown_QueryInterface(IUnknown *iface, REFIID iid, void **out) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 142de9bce9f..0f7d945ba37 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -43,6 +43,29 @@ bool array_reserve(void **elements, size_t *capacity, size_t count, size_t size) #define MEDIATIME_FROM_BYTES(x) ((LONGLONG)(x) * 10000000) +static inline BOOL is_mf_video_area_empty(const MFVideoArea *area) +{ + return !area->OffsetX.value && !area->OffsetY.value && !area->Area.cx && !area->Area.cy; +} + +static inline void get_mf_video_content_rect(const MFVideoInfo *info, RECT *rect) +{ + if (!is_mf_video_area_empty(&info->MinimumDisplayAperture)) + { + rect->left = info->MinimumDisplayAperture.OffsetX.value; + rect->top = info->MinimumDisplayAperture.OffsetY.value; + rect->right = rect->left + info->MinimumDisplayAperture.Area.cx; + rect->bottom = rect->top + info->MinimumDisplayAperture.Area.cy; + } + else + { + rect->left = 0; + rect->top = 0; + rect->right = info->dwWidth; + rect->bottom = info->dwHeight; + } +} + struct wg_sample_queue; HRESULT wg_sample_queue_create(struct wg_sample_queue **out); diff --git a/dlls/winegstreamer/video_processor.c b/dlls/winegstreamer/video_processor.c index 5555cef8303..3cb2c1a5bc6 100644 --- a/dlls/winegstreamer/video_processor.c +++ b/dlls/winegstreamer/video_processor.c @@ -91,6 +91,26 @@ struct video_processor IMFVideoSampleAllocatorEx *allocator; }; +static void update_video_aperture(MFVideoInfo *input_info, MFVideoInfo *output_info) +{ + RECT input_rect, output_rect; + + get_mf_video_content_rect(input_info, &input_rect); + get_mf_video_content_rect(output_info, &output_rect); + + if (!EqualRect(&input_rect, &output_rect)) + { + FIXME("Mismatched content size %s vs %s\n", wine_dbgstr_rect(&input_rect), + wine_dbgstr_rect(&output_rect)); + } + + input_info->MinimumDisplayAperture.OffsetX.value = input_rect.left; + input_info->MinimumDisplayAperture.OffsetY.value = input_rect.top; + input_info->MinimumDisplayAperture.Area.cx = input_rect.right - input_rect.left; + input_info->MinimumDisplayAperture.Area.cy = input_rect.bottom - input_rect.top; + output_info->MinimumDisplayAperture = input_info->MinimumDisplayAperture; +} + static HRESULT normalize_media_types(BOOL bottom_up, IMFMediaType **input_type, IMFMediaType **output_type) { MFVIDEOFORMAT *input_format, *output_format; @@ -114,6 +134,8 @@ static HRESULT normalize_media_types(BOOL bottom_up, IMFMediaType **input_type, if (bottom_up && normalize_output) output_format->videoInfo.VideoFlags |= MFVideoFlag_BottomUpLinearRep; + update_video_aperture(&input_format->videoInfo, &output_format->videoInfo); + if (FAILED(hr = MFCreateVideoMediaType(input_format, (IMFVideoMediaType **)input_type))) goto done; if (FAILED(hr = MFCreateVideoMediaType(output_format, (IMFVideoMediaType **)output_type)))

1 0

Rémi Bernon : winegstreamer: Respect video format padding for input buffers too.
by Alexandre Julliard 03 Jul '24

03 Jul '24

Module: wine Branch: master Commit: 39ad87840a3f383421a89dd93ba69b7bdfd159db URL: https://gitlab.winehq.org/wine/wine/-/commit/39ad87840a3f383421a89dd93ba69b… Author: Rémi Bernon <rbernon(a)codeweavers.com> Date: Tue Jul 2 21:29:58 2024 +0200 winegstreamer: Respect video format padding for input buffers too. --- dlls/mf/tests/transform.c | 50 ++++++++++++++------------------------ dlls/winegstreamer/unix_private.h | 5 ++++ dlls/winegstreamer/wg_media_type.c | 6 +++++ dlls/winegstreamer/wg_transform.c | 18 ++++++++++++++ 4 files changed, 47 insertions(+), 32 deletions(-)

1 0

Piotr Caban : xcopy: Exit after displaying help message.
by Alexandre Julliard 03 Jul '24

03 Jul '24

Module: wine Branch: master Commit: 89fc06a703682c3b7fd7352e6800205c1df6e8fd URL: https://gitlab.winehq.org/wine/wine/-/commit/89fc06a703682c3b7fd7352e680020… Author: Piotr Caban <piotr(a)codeweavers.com> Date: Wed Jul 3 11:56:15 2024 +0200 xcopy: Exit after displaying help message. --- programs/xcopy/xcopy.c | 11 +++-------- programs/xcopy/xcopy.h | 1 - 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 1d003007c0b..57adf1d6b84 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -844,8 +844,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, } break; case '?': XCOPY_wprintf(XCOPY_LoadMessage(STRING_HELP)); - rc = RC_HELP; - goto out; + exit(RC_OK); case 'V': WINE_FIXME("ignoring /V\n"); break; @@ -1098,12 +1097,8 @@ int __cdecl wmain (int argc, WCHAR *argvW[]) * Parse the command line */ if ((rc = XCOPY_ParseCommandLine(suppliedsource, supplieddestination, - &flags)) != RC_OK) { - if (rc == RC_HELP) - return RC_OK; - else - return rc; - } + &flags)) != RC_OK) + return rc; /* Trace out the supplied information */ WINE_TRACE("Supplied parameters:\n"); diff --git a/programs/xcopy/xcopy.h b/programs/xcopy/xcopy.h index 9c6ee1176f5..1ed9e20c45b 100644 --- a/programs/xcopy/xcopy.h +++ b/programs/xcopy/xcopy.h @@ -26,7 +26,6 @@ #define RC_CTRLC 2 #define RC_INITERROR 4 #define RC_WRITEERROR 5 -#define RC_HELP 6 #define OPT_ASSUMEDIR 0x00000001 #define OPT_RECURSIVE 0x00000002

1 0

Piotr Caban : xcopy: Exit on invalid command line argument.
by Alexandre Julliard 03 Jul '24

03 Jul '24

Module: wine Branch: master Commit: f5448260505bcb5078c5016287f44edfc52fef7e URL: https://gitlab.winehq.org/wine/wine/-/commit/f5448260505bcb5078c5016287f44e… Author: Piotr Caban <piotr(a)codeweavers.com> Date: Wed Jul 3 10:55:43 2024 +0200 xcopy: Exit on invalid command line argument. --- programs/xcopy/xcopy.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 57adf1d6b84..202ce903407 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -703,22 +703,22 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, { DWORD flags = *pflags; WCHAR *cmdline, *word, *end, *next; - int rc = RC_INITERROR; + int rc; cmdline = _wcsdup(GetCommandLineW()); if (cmdline == NULL) - return rc; + exit(RC_INITERROR); /* Skip first arg, which is the program name */ if ((rc = find_end_of_word(cmdline, &word)) != RC_OK) - goto out; + exit(rc); word = skip_whitespace(word); while (*word) { WCHAR first; if ((rc = find_end_of_word(word, &end)) != RC_OK) - goto out; + exit(rc); next = skip_whitespace(end); first = word[0]; @@ -734,7 +734,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, lstrcpyW(supplieddestination, word); } else { XCOPY_wprintf(XCOPY_LoadMessage(STRING_INVPARMS)); - goto out; + exit(RC_INITERROR); } } else { /* Process all the switch options @@ -772,7 +772,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, &word[1], 8, L"EXCLUDE:", -1) == CSTR_EQUAL) { if (XCOPY_ProcessExcludeList(&word[9])) { XCOPY_FailMessage(ERROR_INVALID_PARAMETER); - goto out; + exit(RC_INITERROR); } else { flags |= OPT_EXCLUDELIST; @@ -831,7 +831,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, wine_dbgstr_w(datestring), wine_dbgstr_w(timestring)); } else { XCOPY_FailMessage(ERROR_INVALID_PARAMETER); - goto out; + exit(RC_INITERROR); } } else { flags |= OPT_DATENEWER; @@ -851,7 +851,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, default: WINE_TRACE("Unhandled parameter '%s'\n", wine_dbgstr_w(word)); XCOPY_wprintf(XCOPY_LoadMessage(STRING_INVPARM), word); - goto out; + exit(RC_INITERROR); } /* Unless overridden above, skip over the '/' and the first character */ @@ -876,11 +876,7 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, lstrcpyW(supplieddestination, L"."); *pflags = flags; - rc = RC_OK; - - out: - free(cmdline); - return rc; + return RC_OK; }

1 0

Piotr Caban : xcopy: Strip quotes only from source and destination arguments.
by Alexandre Julliard 03 Jul '24

03 Jul '24

Module: wine Branch: master Commit: eab62c31f3a935eded6750db58f594384d3fe525 URL: https://gitlab.winehq.org/wine/wine/-/commit/eab62c31f3a935eded6750db58f594… Author: Piotr Caban <piotr(a)codeweavers.com> Date: Wed Jul 3 11:08:38 2024 +0200 xcopy: Strip quotes only from source and destination arguments. --- programs/xcopy/tests/xcopy.c | 5 +++++ programs/xcopy/xcopy.c | 19 ++++++++----------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/programs/xcopy/tests/xcopy.c b/programs/xcopy/tests/xcopy.c index fa3d5328a1b..2745ead97a5 100644 --- a/programs/xcopy/tests/xcopy.c +++ b/programs/xcopy/tests/xcopy.c @@ -106,6 +106,11 @@ static void test_parms_syntax(void) ok(GetFileAttributesA("xcopytest2") != INVALID_FILE_ATTRIBUTES, "xcopy failed to copy empty directory\n"); RemoveDirectoryA("xcopytest2"); + + rc = runcmd("xcopy xcopytest xcopytest2\\ /D/S/\"E\""); + ok(rc == 4, "xcopy /D/S/\"E\" test failed rc=%lu\n", rc); + ok(GetFileAttributesA("xcopytest2") == INVALID_FILE_ATTRIBUTES, + "xcopy copied empty directory incorrectly\n"); } static void test_keep_attributes(void) diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 202ce903407..072d44728a3 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -684,18 +684,17 @@ static int find_end_of_word(const WCHAR *word, WCHAR **end) } /* Remove all double quotes from a word */ -static void strip_quotes(WCHAR *word, WCHAR **end) +static void strip_quotes(WCHAR *word) { - WCHAR *rp, *wp; - for (rp = word, wp = word; *rp != '\0'; rp++) { - if (*rp == '"') + WCHAR *wp; + for (wp = word; *word != '\0'; word++) { + if (*word == '"') continue; - if (wp < rp) - *wp = *rp; + if (wp < word) + *wp = *word; wp++; } *wp = '\0'; - *end = wp; } static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, @@ -716,18 +715,16 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, while (*word) { - WCHAR first; if ((rc = find_end_of_word(word, &end)) != RC_OK) exit(rc); next = skip_whitespace(end); - first = word[0]; *end = '\0'; - strip_quotes(word, &end); WINE_TRACE("Processing Arg: '%s'\n", wine_dbgstr_w(word)); /* First non-switch parameter is source, second is destination */ - if (first != '/') { + if (word[0] != '/') { + strip_quotes(word); if (suppliedsource[0] == 0x00) { lstrcpyW(suppliedsource, word); } else if (supplieddestination[0] == 0x00) {

1 0

Piotr Caban : xcopy: Introduce get_arg helper that duplicates first argument to new string.
by Alexandre Julliard 03 Jul '24

03 Jul '24

Module: wine Branch: master Commit: 28831f9df951c0c7e06feda99c48693df24e48c1 URL: https://gitlab.winehq.org/wine/wine/-/commit/28831f9df951c0c7e06feda99c4869… Author: Piotr Caban <piotr(a)codeweavers.com> Date: Wed Jul 3 11:35:12 2024 +0200 xcopy: Introduce get_arg helper that duplicates first argument to new string. --- programs/xcopy/xcopy.c | 67 +++++++++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 072d44728a3..715fd8f167b 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -647,7 +647,7 @@ static inline BOOL is_whitespace(WCHAR c) return c == ' ' || c == '\t'; } -static WCHAR *skip_whitespace(WCHAR *p) +static const WCHAR *skip_whitespace(const WCHAR *p) { for (; *p && is_whitespace(*p); p++); return p; @@ -662,10 +662,12 @@ static inline BOOL is_digit(WCHAR c) that lacks the escaped-quote logic of build_argv(), because literal double quotes are illegal in any of its arguments. Example: 'XCOPY "c:\DIR A" "c:DIR B\"' is OK. */ -static int find_end_of_word(const WCHAR *word, WCHAR **end) +static int get_arg(const WCHAR **cmdline, WCHAR **arg) { + const WCHAR *ptr = *cmdline; BOOL in_quotes = FALSE; - const WCHAR *ptr = word; + int len; + for (;;) { for (; *ptr != '\0' && *ptr != '"' && (in_quotes || !is_whitespace(*ptr)); ptr++); @@ -679,7 +681,15 @@ static int find_end_of_word(const WCHAR *word, WCHAR **end) if (*ptr == '\0' || (!in_quotes && is_whitespace(*ptr))) break; } - *end = (WCHAR*)ptr; + + len = ptr - *cmdline; + *arg = malloc((len + 1) * sizeof(WCHAR)); + if (!*arg) + return RC_INITERROR; + memcpy(*arg, *cmdline, len * sizeof(WCHAR)); + (*arg)[len] = 0; + + *cmdline = skip_whitespace(ptr); return RC_OK; } @@ -701,25 +711,20 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, WCHAR *supplieddestination, DWORD *pflags) { DWORD flags = *pflags; - WCHAR *cmdline, *word, *end, *next; + const WCHAR *cmdline; + WCHAR *word; int rc; - cmdline = _wcsdup(GetCommandLineW()); - if (cmdline == NULL) - exit(RC_INITERROR); - + cmdline = GetCommandLineW(); /* Skip first arg, which is the program name */ - if ((rc = find_end_of_word(cmdline, &word)) != RC_OK) + if ((rc = get_arg(&cmdline, &word)) != RC_OK) exit(rc); - word = skip_whitespace(word); + free(word); - while (*word) + while (*cmdline) { - if ((rc = find_end_of_word(word, &end)) != RC_OK) + if ((rc = get_arg(&cmdline, &word)) != RC_OK) exit(rc); - - next = skip_whitespace(end); - *end = '\0'; WINE_TRACE("Processing Arg: '%s'\n", wine_dbgstr_w(word)); /* First non-switch parameter is source, second is destination */ @@ -739,12 +744,12 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, but tests show it is done for each src file regardless of the destination */ int skip=0; - WCHAR *rest; + WCHAR *p = word, *rest; - while (word[0]) { + while (*p) { rest = NULL; - switch (toupper(word[1])) { + switch (toupper(p[1])) { case 'I': flags |= OPT_ASSUMEDIR; break; case 'S': flags |= OPT_RECURSIVE; break; case 'Q': flags |= OPT_QUIET; break; @@ -766,15 +771,15 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, /* E can be /E or /EXCLUDE */ case 'E': if (CompareStringW(LOCALE_USER_DEFAULT, NORM_IGNORECASE | SORT_STRINGSORT, - &word[1], 8, L"EXCLUDE:", -1) == CSTR_EQUAL) { - if (XCOPY_ProcessExcludeList(&word[9])) { + &p[1], 8, L"EXCLUDE:", -1) == CSTR_EQUAL) { + if (XCOPY_ProcessExcludeList(&p[9])) { XCOPY_FailMessage(ERROR_INVALID_PARAMETER); exit(RC_INITERROR); } else { flags |= OPT_EXCLUDELIST; /* Do not support concatenated switches onto exclude lists yet */ - rest = end; + rest = p + wcslen(p); } } else { flags |= OPT_EMPTYDIR | OPT_RECURSIVE; @@ -782,9 +787,9 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, break; /* D can be /D or /D: */ - case 'D': if (word[2]==':' && is_digit(word[3])) { + case 'D': if (p[2]==':' && is_digit(p[3])) { SYSTEMTIME st; - WCHAR *pos = &word[3]; + WCHAR *pos = &p[3]; BOOL isError = FALSE; memset(&st, 0x00, sizeof(st)); @@ -835,9 +840,9 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, } break; - case '-': if (toupper(word[2])=='Y') { + case '-': if (toupper(p[2])=='Y') { flags &= ~OPT_NOPROMPT; - rest = &word[3]; /* Skip over 3 characters */ + rest = &p[3]; /* Skip over 3 characters */ } break; case '?': XCOPY_wprintf(XCOPY_LoadMessage(STRING_HELP)); @@ -846,13 +851,13 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, WINE_FIXME("ignoring /V\n"); break; default: - WINE_TRACE("Unhandled parameter '%s'\n", wine_dbgstr_w(word)); - XCOPY_wprintf(XCOPY_LoadMessage(STRING_INVPARM), word); + WINE_TRACE("Unhandled parameter '%s'\n", wine_dbgstr_w(p)); + XCOPY_wprintf(XCOPY_LoadMessage(STRING_INVPARM), p); exit(RC_INITERROR); } /* Unless overridden above, skip over the '/' and the first character */ - if (rest == NULL) rest = &word[2]; + if (rest == NULL) rest = &p[2]; /* By now, rest should point either to the null after the switch, or the beginning of the next switch if there @@ -861,11 +866,11 @@ static int XCOPY_ParseCommandLine(WCHAR *suppliedsource, WINE_FIXME("Unexpected characters found and ignored '%s'\n", wine_dbgstr_w(rest)); skip=1; } else { - word = rest; + p = rest; } } } - word = next; + free(word); } /* Default the destination if not supplied */

1 0

Piotr Caban : xcopy: Handle switch options concatenated with path.
by Alexandre Julliard 03 Jul '24

03 Jul '24

Module: wine Branch: master Commit: c9314151d9a8568b0b18a48cfa7197bb9f0823ff URL: https://gitlab.winehq.org/wine/wine/-/commit/c9314151d9a8568b0b18a48cfa7197… Author: Piotr Caban <piotr(a)codeweavers.com> Date: Thu Jun 27 08:47:52 2024 +0200 xcopy: Handle switch options concatenated with path. --- programs/xcopy/tests/xcopy.c | 2 +- programs/xcopy/xcopy.c | 17 ++++++----------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/programs/xcopy/tests/xcopy.c b/programs/xcopy/tests/xcopy.c index 2745ead97a5..4c51f941dfc 100644 --- a/programs/xcopy/tests/xcopy.c +++ b/programs/xcopy/tests/xcopy.c @@ -101,7 +101,7 @@ static void test_parms_syntax(void) ok(GetFileAttributesA("xcopytest2") == INVALID_FILE_ATTRIBUTES, "xcopy copied empty directory incorrectly\n"); - rc = runcmd("xcopy /D/S/E xcopytest xcopytest2\\"); + rc = runcmd("xcopy xcopytest xcopytest2\\/D/S/E"); ok(rc == 0, "xcopy /D/S/E test failed rc=%lu\n", rc); ok(GetFileAttributesA("xcopytest2") != INVALID_FILE_ATTRIBUTES, "xcopy failed to copy empty directory\n"); diff --git a/programs/xcopy/xcopy.c b/programs/xcopy/xcopy.c index 715fd8f167b..84f2d312b6f 100644 --- a/programs/xcopy/xcopy.c +++ b/programs/xcopy/xcopy.c @@ -665,21 +665,16 @@ static inline BOOL is_digit(WCHAR c) static int get_arg(const WCHAR **cmdline, WCHAR **arg) { const WCHAR *ptr = *cmdline; - BOOL in_quotes = FALSE; int len; - for (;;) { - for (; *ptr != '\0' && *ptr != '"' && - (in_quotes || !is_whitespace(*ptr)); ptr++); + if (*ptr == '/') ptr++; + while (*ptr && !is_whitespace(*ptr) && *ptr != '/') { if (*ptr == '"') { - in_quotes = !in_quotes; - ptr++; + while (*ptr && *ptr != '"') ptr++; + /* Odd number of double quotes is illegal for XCOPY */ + if (!*ptr) return RC_INITERROR; } - /* Odd number of double quotes is illegal for XCOPY */ - if (in_quotes && *ptr == '\0') - return RC_INITERROR; - if (*ptr == '\0' || (!in_quotes && is_whitespace(*ptr))) - break; + ptr++; } len = ptr - *cmdline;

1 0