August 2025 - wine-gitlab

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

19 Aug '25

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) 19 Aug '25

19 Aug '25

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) 19 Aug '25

19 Aug '25

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) 19 Aug '25

19 Aug '25

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) 19 Aug '25

19 Aug '25

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) 19 Aug '25

19 Aug '25

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) 19 Aug '25

19 Aug '25

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) 19 Aug '25

19 Aug '25

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

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

19 Aug '25

Esme Povirk (@madewokherd) commented about dlls/sane.ds/cfg.c: > + > + new_content = NULL; > + found = FALSE; > + > + sprintf(search, "%s=", option->name); > + if (content) > + { > + CHAR* lines[1000] = {0}; > + int count = 0; > + CHAR* context = NULL; > + size_t new_size = 0; > + CHAR* line; > + > + line = strtok_s(content, "\n", &context); > + > + while (line && count < 1000) Please don't duplicate the buffer length. You can use the `ARRAY_SIZE` macro. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113164

1 0

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

19 Aug '25

Esme Povirk (@madewokherd) commented about dlls/sane.ds/cfg.c: > + sprintf(buffer, "%s=%s\n", (option->name), option->value.bool_val ? "true" : "false"); > + break; > + } > + > + size = GetFileSize(hFile, NULL); > + content = (CHAR*)malloc(size + sizeof(CHAR)); > + > + ReadFile(hFile, content, size, &read, NULL); > + content[size / sizeof(CHAR)] = '\0'; > + CloseHandle(hFile); > + > + new_content = NULL; > + found = FALSE; > + > + sprintf(search, "%s=", option->name); > + if (content) This is checking whether the malloc succeeded after the allocated value has already been used. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8778#note_113163

1 0

2025

2024

2023

2022

wine-gitlab August 2025