Re: [PATCH 2/2] regedit: Handle comment lines with leading spaces and tabs
Hugh McMaster <hugh.mcmaster(a)outlook.com> writes:
Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> --- programs/regedit/regproc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/programs/regedit/regproc.c b/programs/regedit/regproc.c index 131149f..bce551b 100644 --- a/programs/regedit/regproc.c +++ b/programs/regedit/regproc.c @@ -749,6 +749,7 @@ static char *get_lineA(FILE *fp) next = line; continue; } + while (*line == ' ' || *line == '\t') line++; if (*line == ';' || *line == '#') { line = next;
This is going to affect non-comment lines too, is that what you want? If so, the commit message is a bit misleading. -- Alexandre Julliard julliard(a)winehq.org
On Friday, 31 March 2017 4:43 AM, Alexandre Julliard wrote:
Hugh McMaster writes:
--- a/programs/regedit/regproc.c +++ b/programs/regedit/regproc.c @@ -749,6 +749,7 @@ static char *get_lineA(FILE *fp) next = line; continue; } + while (*line == ' ' || *line == '\t') line++; if (*line == ';' || *line == '#') { line = next;
This is going to affect non-comment lines too, is that what you want? If so, the commit message is a bit misleading.
I guess it depends on how you think we should handle such whitespace. parse_file_header() checks for whitespace before processing the file header. We don't bother checking in processRegEntry() or processRegEntry31(). On Windows, all lines in registry files can have leading spaces and tabs. The exception is Windows 3.1, which doesn't allow leading whitespace at all. In Wine, we ignore any registry line with leading whitespace. So importing a file like the following will fail. (In case it's not clear, there are two spaces before the opening square bracket.) REGEDIT4 [HKEY_CURRENT_USER\Software\Wine\regedit_test] "Value"="Data" One option is to move the check for ';' and '#' to processRegEntry(), but that would require A->W line conversion first. We would then add an appropriate while loop in that function, handling all line cases. Otherwise, we could add the while loops into get_lineA/W() and remove the whitespace check from parse_file_header(), as it would be redundant. I'm open to other ideas as well. Thanks for raising this. -- Hugh McMaster
Hugh McMaster <hugh.mcmaster(a)outlook.com> writes:
I guess it depends on how you think we should handle such whitespace. parse_file_header() checks for whitespace before processing the file header. We don't bother checking in processRegEntry() or processRegEntry31().
On Windows, all lines in registry files can have leading spaces and tabs. The exception is Windows 3.1, which doesn't allow leading whitespace at all.
In Wine, we ignore any registry line with leading whitespace. So importing a file like the following will fail. (In case it's not clear, there are two spaces before the opening square bracket.)
REGEDIT4 [HKEY_CURRENT_USER\Software\Wine\regedit_test] "Value"="Data"
One option is to move the check for ';' and '#' to processRegEntry(), but that would require A->W line conversion first. We would then add an appropriate while loop in that function, handling all line cases.
Otherwise, we could add the while loops into get_lineA/W() and remove the whitespace check from parse_file_header(), as it would be redundant.
You can also ignore white space before comments, but still return it for other lines, if there is a case where the callers would want that. Otherwise getting rid of whitespace right away is fine. A few test cases with leading whitespace may be a good idea. -- Alexandre Julliard julliard(a)winehq.org
On Friday, 31 March 2017 11:07 PM, Alexandre Julliard wrote:
You can also ignore white space before comments, but still return it for other lines, if there is a case where the callers would want that. Otherwise getting rid of whitespace right away is fine. A few test cases with leading whitespace may be a good idea.
Somewhat strangely, we actually call isspaceW() in processSetValue() before we get the registry value name. But it is already too late. We need to check for whitespace in processRegEntry() or earlier; otherwise, the import fails. So I think handling whitespace in get_lineA/W() will be far more efficient and easier to maintain. But to be certain, I'll add some tests with whitespace variations first. Thanks for the advice. -- Hugh McMaster
participants (2)
-
Alexandre Julliard -
Hugh McMaster