February 2025 - Wine-GitLab

Re: [PATCH v10 0/1] MR7322: programs/cmd: Implement ability to abort lengthy directory operations via Ctrl-C.
by Joe Souza (＠JoeS209) Feb. 16, 2025

Feb. 16, 2025

On Sun Feb 16 16:37:14 2025 +0000, eric pouech wrote: > this shouldn't set errorlevel but return_code > and likely using ERROR_FILE_NOT_FOUND to mark that listing was empty My head is starting to spin here. That is existing code that I haven't touched. I'll fix along with the other issues though... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7322#note_94829

1 0

Re: [PATCH v2 0/1] MR7339: ntdll: implement create_logical_proc_info on FreeBSD.
by Brendan Shanks (＠bshanks) Feb. 16, 2025

Feb. 16, 2025

Another possibility is to use `libbsdxml`, a renamed expat that FreeBSD includes (although the man page says that “The libbsdxml library is intended to be used within the FreeBSD base system only. Use of the libbsdxml library for other purposes is not supported and discouraged.” But expat’s API is quite stable, and avahi also has the option to use bsdxml.) Otherwise, maybe just depend on a system-installed expat or libxml2 on FreeBSD? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7339#note_94826

1 0

Re: [PATCH v10 0/1] MR7322: programs/cmd: Implement ability to abort lengthy directory operations via Ctrl-C.
by eric pouech (＠epo) Feb. 16, 2025

Feb. 16, 2025

globally it looks better a couple of stylistic remarks, but you need to go further for not using errorlevel inside the function and its helper perhaps, it's worthwhile to split the patch in two: * first one to no longer use error level internally, adapt list_directory prototype... (in the details, it's fine to use return errorlevel = ERROR\_\* while parsing the arguments in case of error, but when this is done all the rest of the code should use only the return_code (and set errorlevel when exiting WCMD_dir)) * the second one to plug in the ctrl-c handling and breaking from the various loops -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7322#note_94825

1 0

Re: [PATCH v10 0/1] MR7322: programs/cmd: Implement ability to abort lengthy directory operations via Ctrl-C.
by eric pouech (＠epo) Feb. 16, 2025

Feb. 16, 2025

eric pouech (@epo) commented about programs/cmd/directory.c: > * FIXME: Assumes 24-line display for the /P qualifier. > */ > > -static DIRECTORY_STACK *WCMD_list_directory (DIRECTORY_STACK *inputparms, int level) { > +static DIRECTORY_STACK *WCMD_list_directory (DIRECTORY_STACK *inputparms, int level, RETURN_CODE *return_code) { nitpick: IMO it's more readable to return a RETURN_CODE, and pass a DIRECTORY_STACK\*\* to update the pointer (you likely did it this way to minimize change, but let's take the opportunity of a change to sanitize a bit the current spaghetti code) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7322#note_94822

1 0

Re: [PATCH v10 0/1] MR7322: programs/cmd: Implement ability to abort lengthy directory operations via Ctrl-C.
by eric pouech (＠epo) Feb. 16, 2025

Feb. 16, 2025

eric pouech (@epo) commented about programs/cmd/directory.c: > thisDir = tempDir; > } > } > + /* Free remaining structures if we broke out of previous loop due to error. */ > + while (dirStack != NULL) { this likely could be folded in code line 510, by inserting between 510 & 511: if (return_code != NO_ERROR) dirStack = NULL; -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7322#note_94821

1 0

Re: [PATCH v10 0/1] MR7322: programs/cmd: Implement ability to abort lengthy directory operations via Ctrl-C.
by eric pouech (＠epo) Feb. 16, 2025

Feb. 16, 2025

eric pouech (@epo) commented about programs/cmd/directory.c: > } > > /* Trailer Information */ > - if (trailerReqd) { > + if (trailerReqd && return_code == NO_ERROR) { > WCMD_dir_trailer(prevEntry->dirName); > } > > - if (num_empty && !num_with_data) > + if (return_code == STATUS_CONTROL_C_EXIT) actually, we could (later on) have other values as return code... so IMO this should rather be written as: `if (return_code != NO_ERROR || (num_empty && !num_with_data))` ` errorlevel = ERROR_INVALID_FUNCTION;` `else` ` errorlevel = NO_ERROR;` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7322#note_94820

1 0

Re: [PATCH v10 0/1] MR7322: programs/cmd: Implement ability to abort lengthy directory operations via Ctrl-C.
by eric pouech (＠epo) Feb. 16, 2025

Feb. 16, 2025

eric pouech (@epo) commented about programs/cmd/directory.c: > /* Clear any errors from previous invocations, and process it */ > errorlevel = NO_ERROR; > prevEntry = thisEntry; > - thisEntry = WCMD_list_directory (thisEntry, 0); > + thisEntry = WCMD_list_directory (thisEntry, 0, &return_code); > if (errorlevel) this should use return_code not error level -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7322#note_94819

1 0

Re: [PATCH v10 0/1] MR7322: programs/cmd: Implement ability to abort lengthy directory operations via Ctrl-C.
by eric pouech (＠epo) Feb. 16, 2025

Feb. 16, 2025

eric pouech (@epo) commented about programs/cmd/directory.c: > if ((file_total + dir_total == 0) && (level == 0)) { > SetLastError (ERROR_FILE_NOT_FOUND); > WCMD_print_error (); > errorlevel = ERROR_INVALID_FUNCTION; this shouldn't set errorlevel but return_code and likely using ERROR_FILE_NOT_FOUND to mark that listing was empty -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7322#note_94818

1 0

Re: [PATCH v10 0/1] MR7322: programs/cmd: Implement ability to abort lengthy directory operations via Ctrl-C.
by eric pouech (＠epo) Feb. 16, 2025

Feb. 16, 2025

eric pouech (@epo) commented about programs/cmd/directory.c: > WCHAR fname[MAX_PATH]; > WCHAR ext[MAX_PATH]; > unsigned num_empty = 0, num_with_data = 0; > + RETURN_CODE return_code = NO_ERROR; > > errorlevel = NO_ERROR; this should not be necessary (in fact starting from here up the exit of the function, you should only deal with return_code... errorlevel should be set upon exiting) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7322#note_94817

1 0

Re: [PATCH v10 0/1] MR7322: programs/cmd: Implement ability to abort lengthy directory operations via Ctrl-C.
by eric pouech (＠epo) Feb. 16, 2025

Feb. 16, 2025

eric pouech (@epo) commented about programs/cmd/directory.c: > /* Clear any errors from previous invocations, and process it */ > errorlevel = NO_ERROR; this should be return_code -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7322#note_94815

1 0