[Bug 44310] New: WritePrivateProfileString doesn't check for flush errors

List overview All Threads

newer

older

[Bug 24721] New: Explorer++...

[Bug 44213] New: Failure to run...

wine-bugs＠winehq.org

5 Jan 2018 5 Jan '18

8:55 p.m.

https://bugs.winehq.org/show_bug.cgi?id=44310

Bug ID: 44310 Summary: WritePrivateProfileString doesn't check for flush errors Product: Wine Version: 3.0-rc5 Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: minor Priority: P2 Component: kernel32 Assignee: wine-bugs@winehq.org Reporter: development@winomega.com Distribution: ---

Created attachment 60148 --> https://bugs.winehq.org/attachment.cgi?id=60148 test case

The Windows version of WritePrivateProfileString reports an error if flushing to file failed.

See attached test case. It tries to write to Z:\test.ini (which is expected to fail as Z is mapped to /, with no write perms).

Under wine I get no output from the test, and the file isn't created. Failure warnings do show up in +profile channel log:

trace:profile:PROFILE_Open path: L"Z:\test.ini" warn:profile:PROFILE_Open profile file L"Z:\test.ini" not found trace:profile:PROFILE_SetString (L"Section1",L"FirstKey",L"It all worked out OK."): trace:profile:PROFILE_SetString creating key warn:profile:PROFILE_FlushFile could not save profile file L"Z:\test.ini" (error was 5) warn:profile:PROFILE_FlushFile could not save profile file L"Z:\test.ini" (error was 5)

Under Windows XP (where Z: drive doesn't exist), I get "failed with error 3" message.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

Show replies by date

wine-bugs＠winehq.org

5 Jan 5 Jan

8:56 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #1 from Omega Software development@winomega.com --- Created attachment 60149 --> https://bugs.winehq.org/attachment.cgi?id=60149 fix for current git

Attached patch fixes the problem in current git, by checking for PROFILE_FlushFile result (instead of just PROFILE_SetString).

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

8:57 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

Omega Software development@winomega.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch, testcase

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

9 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #2 from Omega Software development@winomega.com --- For the records, this bug also affects Delphi applications that rely on TIniFile: WriteString should be rising an exception when it is unable to write, but it doesn't.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

6 Jan 6 Jan

11:02 a.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

Fabian Maurer dark.shadow4@web.de changed:

--- Comment #3 from Fabian Maurer dark.shadow4@web.de --- Good catch, this is indeed problematic. You probably want to add a test case (should be easy enough) and then sent it to the mailing list once code-freeze is over.

While you're at it, there are more places where PROFILE_FlushFile is called. Do these show the same erroneous behavior? Like in "WritePrivateProfileSectionW", "WritePrivateProfileStructW"

Site not, I don't like chaining two methods that have side effects with "&&" like that. I'd probably prefer ------ ret = PROFILE_SetString( section, entry, string, FALSE); ret &= PROFILE_FlushFile(); ------ or ------ ret = PROFILE_SetString( section, entry, string, FALSE); if(ret) ret = PROFILE_FlushFile(); ------ But I don't know about the coding style, just wanted to note.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

9 Jan 9 Jan

11:57 a.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #4 from Omega Software development@winomega.com --- Hi!

(In reply to Fabian Maurer from comment #3)

...

Good catch, this is indeed problematic. You probably want to add a test case (should be easy enough) and then sent it to the mailing list once code-freeze is over.

I would, but I'm not sure what directory can be used in the test suite that is guaranteed not to be writable. Any idea?

...

While you're at it, there are more places where PROFILE_FlushFile is called. Do these show the same erroneous behavior? Like in "WritePrivateProfileSectionW", "WritePrivateProfileStructW"

They do. I've extended both the test case and the patch (attaching).

...

Site not, I don't like chaining two methods that have side effects with "&&" like that. I'd probably prefer

ret = PROFILE_SetString( section, entry, string, FALSE); ret &= PROFILE_FlushFile();

Yes, much more readable this way.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

11:58 a.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

Omega Software development@winomega.com changed:

--- Comment #5 from Omega Software development@winomega.com --- Created attachment 60198 --> https://bugs.winehq.org/attachment.cgi?id=60198 fix for current git

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

11:58 a.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

Omega Software development@winomega.com changed:

--- Comment #6 from Omega Software development@winomega.com --- Created attachment 60199 --> https://bugs.winehq.org/attachment.cgi?id=60199 test case

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

9:52 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #7 from Fabian Maurer dark.shadow4@web.de --- Created attachment 60203 --> https://bugs.winehq.org/attachment.cgi?id=60203 test cases

That's a good question actually. You most likely will have to create the needed setting yourself, i.e. make a location where it can't save the profile to.

Setting the readonly attribute on the file is not enough, since wine already checks this. But you can use ACL to forbid everyone to write to the folder! I found that to be a though one, I attached two tests that you can use, if you want. It should already work fine, but I kinda created them in a hurry, so no warranty.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

10 Jan 10 Jan

10:48 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

Omega Software development@winomega.com changed:

--- Comment #8 from Omega Software development@winomega.com --- Created attachment 60216 --> https://bugs.winehq.org/attachment.cgi?id=60216 fix + testcase

Thanks! That was really helpful. I've adapted your directory test function to use the same dummy test values and generate similar output as the other tests in the same file.

Here's the new patch including test case.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

11 Jan 11 Jan

4:48 a.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #9 from Fabian Maurer dark.shadow4@web.de ---

...

ok(ret == FALSE, "Expected FALSE, got %d\n", ret);

I don't think you need it like that, since were dealing with a boolean here. When it's not FALSE, it must be TRUE.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

8:08 a.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #10 from Omega Software development@winomega.com --- That's how it's done in the rest of dlls/kernel32/tests/profile.c. See:

ret = WritePrivateProfileStringA(NULL, "key", "string", path); ok(ret == FALSE, "Expected FALSE, got %d\n", ret); ok(GetLastError() == ERROR_FILE_NOT_FOUND || -- ret = WritePrivateProfileStringA(NULL, "key", "string", path); ok(ret == FALSE, "Expected FALSE, got %d\n", ret); ok(GetLastError() == ERROR_FILE_NOT_FOUND || -- ret = WritePrivateProfileStringA("App", "key", "string", ""); ok(ret == FALSE, "Expected FALSE, got %d\n", ret); ok(GetLastError() == ERROR_ACCESS_DENIED ||

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

6:22 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #11 from Fabian Maurer dark.shadow4@web.de --- Oh I see... Well, I'd still say it's superfluous.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

7:10 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #12 from Alexandre Julliard julliard@winehq.org --- (In reply to Fabian Maurer from comment #11)

...

Oh I see... Well, I'd still say it's superfluous.

It probably doesn't matter in this case, but note that there's nothing that says that a boolean must be TRUE, any non-zero value would do. So printing it is not necessarily useless.

For the same reason, your &= trick is not a good idea.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

7:33 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #13 from Fabian Maurer dark.shadow4@web.de ---

...

It probably doesn't matter in this case, but note that there's nothing that says that a boolean must be TRUE, any non-zero value would do. So printing it is not necessarily useless.

For the same reason, your &= trick is not a good idea.

Is that "&=" a real concern at this point? Because I think it makes it easier to read while it's pretty easy to check that it's safe.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

7:51 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #14 from Alexandre Julliard julliard@winehq.org --- (In reply to Fabian Maurer from comment #13)

...

...
It probably doesn't matter in this case, but note that there's nothing that says that a boolean must be TRUE, any non-zero value would do. So printing it is not necessarily useless.

For the same reason, your &= trick is not a good idea.

Is that "&=" a real concern at this point? Because I think it makes it easier to read while it's pretty easy to check that it's safe.

It may look easier on the surface, but using an incorrect construct is actually more confusing for people reading the code.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

8:03 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #15 from Fabian Maurer dark.shadow4@web.de ---

...

It may look easier on the surface, but using an incorrect construct is actually more confusing for people reading the code.

I see, thanks for explaining.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

9:18 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #16 from Omega Software development@winomega.com --- Which way to go, then? Should we use this:

if(ret) ret = PROFILE_FlushFile();

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

9:43 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #17 from Alexandre Julliard julliard@winehq.org --- (In reply to Omega Software from comment #16)

...

Which way to go, then? Should we use this:

if(ret) ret = PROFILE_FlushFile();

?

Yes this one is fine.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

10:05 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

Omega Software development@winomega.com changed:

--- Comment #18 from Omega Software development@winomega.com --- Created attachment 60231 --> https://bugs.winehq.org/attachment.cgi?id=60231 fix + testcase

Ok here is it.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

31 Jan 31 Jan

3:37 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #19 from Omega Software development@winomega.com --- Hi I sent the patch to https://www.winehq.org/pipermail/wine-devel/2018-January/121762.html

I assume it is waiting for approval now. Please tell me if you need anything else.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

1 Feb 1 Feb

1:13 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #20 from Omega Software development@winomega.com --- Based on instructions from https://wiki.winehq.org/Developer_FAQ#I_sent_a_patch.2C_but_it_got_ignored._... I'm going to submit only the conformance test first, as my first attempt didn't catch Julliard's attention.

Fabian, may I have your permission to put your name in Signed-off-by for this part, along with mine? (the code is mostly yours after all)

Thanks

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

7:33 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #21 from Fabian Maurer dark.shadow4@web.de --- I'm going to submit only the conformance test first, as my first

...

attempt didn't catch Julliard's attention.

Well, it might take some more time, but splitting it into two parts can't harm, I guess. But I'd send both patches at once, as a patchset. Just make sure to mark the tests as todo_wine in the first patch, and remove the todo_wine with the patch that fixes the issue.

...

Fabian, may I have your permission to put your name in Signed-off-by for this part, along with mine? (the code is mostly yours after all)

Sure thing, but please CC me when sending it.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

10:32 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

Fabian Maurer dark.shadow4@web.de changed:

--- Comment #22 from Fabian Maurer dark.shadow4@web.de --- Congrats, you got it in already - no need for resending.

marking fixed by 39899fd11a8ff9228a587adf5e9aeaf2ded93c5c.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

2 Feb 2 Feb

9:02 a.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

--- Comment #23 from Omega Software development@winomega.com --- Excellent! Thanks

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

6:26 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

Alexandre Julliard julliard@winehq.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED

--- Comment #24 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 3.1.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

20 Feb 20 Feb

9:13 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

Michael Stefaniuc mstefani@winehq.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |3.0.x

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

wine-bugs＠winehq.org

14 May 14 May

12:05 p.m.

New subject: [Bug 44310] WritePrivateProfileString doesn't check for flush errors

https://bugs.winehq.org/show_bug.cgi?id=44310

Michael Stefaniuc mstefani@winehq.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|3.0.x |---

--- Comment #25 from Michael Stefaniuc mstefani@winehq.org --- Removing the 3.0.x milestone from bugs included in 3.0.1.

-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.

2431

Age (days ago)

2560

Last active (days ago)

wine-bugs@winehq.org

27 comments

1 participants

tags (0)

participants (1)

wine-bugs＠winehq.org