August 2025 - Wine-GitLab - list.winehq.org

Re: [PATCH v13 0/1] MR8778: sane.ds: Adding a window for loading and saving the scanner configuration
by Esme Povirk (＠madewokherd) Aug. 19, 2025

Aug. 19, 2025

Esme Povirk (@madewokherd) commented about dlls/sane.ds/ui.c: > return FALSE; > } > > +static void set_value(struct option_descriptor* opt, ScannerOption* option) This is called `set_value` but it gets the value? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113174

1 0

Re: [PATCH v13 0/1] MR8778: sane.ds: Adding a window for loading and saving the scanner configuration
by Esme Povirk (＠madewokherd) Aug. 19, 2025

Aug. 19, 2025

Esme Povirk (@madewokherd) commented about dlls/sane.ds/ui.c: > if (rc != TWCC_SUCCESS) > { > ERR("Unable to read number of options\n"); > - return FALSE; > + optcount = gOptCount; What situation causes this failure, and why is using a previous cached value OK? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113172

1 0

Re: [PATCH v13 0/1] MR8778: sane.ds: Adding a window for loading and saving the scanner configuration
by Esme Povirk (＠madewokherd) Aug. 19, 2025

Aug. 19, 2025

Esme Povirk (@madewokherd) commented about dlls/sane.ds/cfg.h: > +{ > + int type; > + CHAR name[64]; > + int optno; > + union > + { > + int int_val; > + BOOL bool_val; > + CHAR str_val[255]; > + int fixed_val; > + } value; > + BOOL is_enabled; > +} ScannerOption; > + > +BOOL save_to_file(WCHAR* path, ScannerOption *option); > +BOOL load_from_file(WCHAR* path, int type, CHAR* name, void* value); This interface seems like it would be inefficient - each setting saved is a complete read and write of the file, and each setting loaded is a complete read. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113173

1 0

Re: [PATCH v13 0/1] MR8778: sane.ds: Adding a window for loading and saving the scanner configuration
by Esme Povirk (＠madewokherd) Aug. 19, 2025

Aug. 19, 2025

Esme Povirk (@madewokherd) commented about dlls/sane.ds/ui.c: > } > > control_len += len + padding; > + optionsExist[cntOptionsExist] = opt.optno; `optionsExist` is not bounds-checked. `cntOptionsExist` is never reset to 0 if `create_options_page` is called multiple times. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113171

1 0

Re: [PATCH v13 0/1] MR8778: sane.ds: Adding a window for loading and saving the scanner configuration
by Esme Povirk (＠madewokherd) Aug. 19, 2025

Aug. 19, 2025

Esme Povirk (@madewokherd) commented about dlls/sane.ds/ui.c: > > +#include "cfg.h" > + > WINE_DEFAULT_DEBUG_CHANNEL(twain); > > #define ID_BASE 0x100 > #define ID_EDIT_BASE 0x1000 > #define ID_STATIC_BASE 0x2000 > +#define ID_APPLY_NOW 0x3021 > + > +#define ID_SAVE_CFG 0x8337 > +#define ID_EDIT_FILENAME 0x8338 > +#define ID_BUTTON_SAVE 0x8339 > +#define ID_BUTTON_LOAD 0x8340 > + > +#define CSIDL_LOCAL_APPDATA 0x001c Why not include the header that defines this? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113170

1 0

Re: [PATCH v13 0/1] MR8778: sane.ds: Adding a window for loading and saving the scanner configuration
by Esme Povirk (＠madewokherd) Aug. 19, 2025

Aug. 19, 2025

Esme Povirk (@madewokherd) commented about dlls/sane.ds/cfg.c: > + break; > + case TYPE_STRING: > + strcpy((CHAR*)value, val_start); > + break; > + case TYPE_BOOL: > + if (!strcmp(val_start, "true")) > + { > + *(BOOL*)value = TRUE; > + } > + else if (!strcmp(val_start, "false")) > + { > + *(BOOL*)value = FALSE; > + } > + else > + { > + return FALSE; This leaks `content`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113169

1 0

Re: [PATCH v13 0/1] MR8778: sane.ds: Adding a window for loading and saving the scanner configuration
by Esme Povirk (＠madewokherd) Aug. 19, 2025

Aug. 19, 2025

Esme Povirk (@madewokherd) commented about dlls/sane.ds/cfg.c: > + CloseHandle(hFile); > + > + sprintf(search, "%s=", name); > + context = NULL; > + line = strtok_s(content, "\n", &context); > + found = FALSE; > + > + while (line) > + { > + if (strstr(line, search) == line) > + { > + CHAR* val_start = line + strlen(search); > + CHAR* space = strchr(val_start, ' '); > + > + if (space) > + *space = '\0'; Ignoring anything after the first space in a string is odd. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113168

1 0

Re: [PATCH v13 0/1] MR8778: sane.ds: Adding a window for loading and saving the scanner configuration
by Esme Povirk (＠madewokherd) Aug. 19, 2025

Aug. 19, 2025

Esme Povirk (@madewokherd) commented about dlls/sane.ds/cfg.c: > + BOOL found; > + HANDLE hFile = CreateFileW( > + path, > + GENERIC_READ, > + 0, NULL, OPEN_EXISTING, > + FILE_ATTRIBUTE_NORMAL, > + NULL > + ); > + > + if (hFile == INVALID_HANDLE_VALUE) > + { > + ERR("CreateFileW returned error: %lu\n", GetLastError()); > + return FALSE; > + } > + > + sprintf(search, "%s=", name); This `sprintf` is done twice. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113167

1 0

Re: [PATCH v13 0/1] MR8778: sane.ds: Adding a window for loading and saving the scanner configuration
by Esme Povirk (＠madewokherd) Aug. 19, 2025

Aug. 19, 2025

Esme Povirk (@madewokherd) commented about dlls/sane.ds/cfg.c: > + while (line && count < 1000) > + { > + if (!found && strstr(line, search) == line) > + { > + lines[count++] = buffer; > + found = TRUE; > + } > + else > + { > + lines[count++] = strdup(line); > + } > + line = strtok_s(NULL, "\n", &context); > + } > + if (!found) > + { > + lines[count++] = buffer; This could write past the end of `lines`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113166

1 0

Re: [PATCH v13 0/1] MR8778: sane.ds: Adding a window for loading and saving the scanner configuration
by Esme Povirk (＠madewokherd) Aug. 19, 2025

Aug. 19, 2025

Esme Povirk (@madewokherd) commented about dlls/sane.ds/cfg.c: > + CHAR* context = NULL; > + size_t new_size = 0; > + CHAR* line; > + > + line = strtok_s(content, "\n", &context); > + > + while (line && count < 1000) > + { > + if (!found && strstr(line, search) == line) > + { > + lines[count++] = buffer; > + found = TRUE; > + } > + else > + { > + lines[count++] = strdup(line); I don't see a reason to make an additional allocation for each line. The original pointer should remain valid until `content` is freed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113165

1 0