mpr: Return correct error code for non network paths and REMOTE_NAME_INFO_LEVEL in WNetGetUniversalName.
From: Michael Müller michael@fds-team.de
Signed-off-by: Michael Müller michael@fds-team.de Signed-off-by: Zebediah Figura z.figura12@gmail.com --- Fixes https://bugs.winehq.org/show_bug.cgi?id=39452.
dlls/mpr/tests/mpr.c | 9 +++------ dlls/mpr/wnet.c | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/dlls/mpr/tests/mpr.c b/dlls/mpr/tests/mpr.c index a6af1b57f8d..1b16369d514 100644 --- a/dlls/mpr/tests/mpr.c +++ b/dlls/mpr/tests/mpr.c @@ -101,25 +101,23 @@ static void test_WNetGetRemoteName(void) info_size = sizeof(buffer); ret = WNetGetUniversalNameA(driveA, REMOTE_NAME_INFO_LEVEL, buffer, &info_size); - todo_wine{ if(drive_type == DRIVE_REMOTE) + todo_wine ok(ret == WN_NO_ERROR, "WNetGetUniversalNameA failed: %08x\n", ret); else ok(ret == WN_NOT_CONNECTED || ret == WN_NO_NET_OR_BAD_PATH, "(%s) WNetGetUniversalNameA gave wrong error: %u\n", driveA, ret); - } ok(info_size == sizeof(buffer), "Got wrong size: %u\n", info_size);
fail_size = 0; ret = WNetGetUniversalNameA(driveA, REMOTE_NAME_INFO_LEVEL, buffer, &fail_size); - todo_wine{ if(drive_type == DRIVE_REMOTE) + todo_wine ok(ret == WN_BAD_VALUE || ret == WN_MORE_DATA, "WNetGetUniversalNameA failed: %08x\n", ret); else ok(ret == WN_NOT_CONNECTED || ret == WN_NO_NET_OR_BAD_PATH, "(%s) WNetGetUniversalNameA gave wrong error: %u\n", driveA, ret); - } ret = WNetGetUniversalNameA(driveA, REMOTE_NAME_INFO_LEVEL, buffer, NULL); todo_wine ok(ret == WN_BAD_POINTER, "WNetGetUniversalNameA failed: %08x\n", ret); @@ -127,13 +125,12 @@ static void test_WNetGetRemoteName(void) ret = WNetGetUniversalNameA(driveA, REMOTE_NAME_INFO_LEVEL, NULL, &info_size);
- todo_wine { if(drive_type == DRIVE_REMOTE) + todo_wine ok(ret == WN_BAD_POINTER || ret == WN_BAD_VALUE, "WNetGetUniversalNameA failed: %08x\n", ret); else ok(ret == WN_NOT_CONNECTED || ret == WN_BAD_VALUE, "(%s) WNetGetUniversalNameA gave wrong error: %u\n", driveA, ret); - }
fail_size = sizeof(driveA) / sizeof(char) - 1; ret = WNetGetUniversalNameA(driveA, REMOTE_NAME_INFO_LEVEL, diff --git a/dlls/mpr/wnet.c b/dlls/mpr/wnet.c index 7b24693e0e8..78759ecb152 100644 --- a/dlls/mpr/wnet.c +++ b/dlls/mpr/wnet.c @@ -2348,7 +2348,7 @@ DWORD WINAPI WNetGetUniversalNameA ( LPCSTR lpLocalPath, DWORD dwInfoLevel, break; } case REMOTE_NAME_INFO_LEVEL: - err = WN_NO_NETWORK; + err = WN_NOT_CONNECTED; break;
default:
Zebediah Figura z.figura12@gmail.com wrote:
From: Michael Müller michael@fds-team.de
Signed-off-by: Michael Müller michael@fds-team.de Signed-off-by: Zebediah Figura z.figura12@gmail.com
If I recall correctly the rules you can't add a sign-off for somebody else without an explicit permission from that person.
On 13/03/18 20:48, Dmitry Timoshkov wrote:
Zebediah Figura z.figura12@gmail.com wrote:
From: Michael Müller michael@fds-team.de
Signed-off-by: Michael Müller michael@fds-team.de Signed-off-by: Zebediah Figura z.figura12@gmail.com
If I recall correctly the rules you can't add a sign-off for somebody else without an explicit permission from that person.
The sign-off wasn't added; it was on the original patch.
Zebediah Figura z.figura12@gmail.com wrote:
From: Michael Müller michael@fds-team.de
Signed-off-by: Michael Müller michael@fds-team.de Signed-off-by: Zebediah Figura z.figura12@gmail.com
If I recall correctly the rules you can't add a sign-off for somebody else without an explicit permission from that person.
The sign-off wasn't added; it was on the original patch.
Still, it's you who is sending this patch, and you need to provide only your own sign-off, since the original author no longer takes any responsibility for this piece of code.
Dmitry Timoshkov dmitry@baikal.ru writes:
Zebediah Figura z.figura12@gmail.com wrote:
From: Michael Müller michael@fds-team.de
Signed-off-by: Michael Müller michael@fds-team.de Signed-off-by: Zebediah Figura z.figura12@gmail.com
If I recall correctly the rules you can't add a sign-off for somebody else without an explicit permission from that person.
The sign-off wasn't added; it was on the original patch.
Still, it's you who is sending this patch, and you need to provide only your own sign-off, since the original author no longer takes any responsibility for this piece of code.
If the patch is not changed, preserving the original sign-off is the right thing to do.
On Wed, Mar 14, 2018 at 11:03:25AM +0100, Alexandre Julliard wrote:
Dmitry Timoshkov dmitry@baikal.ru writes:
Zebediah Figura z.figura12@gmail.com wrote:
From: Michael Müller michael@fds-team.de
Signed-off-by: Michael Müller michael@fds-team.de Signed-off-by: Zebediah Figura z.figura12@gmail.com
If I recall correctly the rules you can't add a sign-off for somebody else without an explicit permission from that person.
The sign-off wasn't added; it was on the original patch.
Still, it's you who is sending this patch, and you need to provide only your own sign-off, since the original author no longer takes any responsibility for this piece of code.
If the patch is not changed, preserving the original sign-off is the right thing to do.
I thought Signed-off-by meant something like, "I agree to help debug this if something goes wrong." It seems wrong to make that statement to wine-devel on someone else's behalf. If I put a sign-off and send it to wine-staging that means something different to me than if I send it to wine-devel.
My understanding was the same as Dmitry's. If the original author wants to sign-off on the patch, they should do it themselves. This also makes the history a little more clear. A patch without a sign-off from the author means it was submitted by someone else.
Andrew
Andrew Eikum aeikum@codeweavers.com writes:
On Wed, Mar 14, 2018 at 11:03:25AM +0100, Alexandre Julliard wrote:
Dmitry Timoshkov dmitry@baikal.ru writes:
Zebediah Figura z.figura12@gmail.com wrote:
From: Michael Müller michael@fds-team.de
Signed-off-by: Michael Müller michael@fds-team.de Signed-off-by: Zebediah Figura z.figura12@gmail.com
If I recall correctly the rules you can't add a sign-off for somebody else without an explicit permission from that person.
The sign-off wasn't added; it was on the original patch.
Still, it's you who is sending this patch, and you need to provide only your own sign-off, since the original author no longer takes any responsibility for this piece of code.
If the patch is not changed, preserving the original sign-off is the right thing to do.
I thought Signed-off-by meant something like, "I agree to help debug this if something goes wrong." It seems wrong to make that statement to wine-devel on someone else's behalf. If I put a sign-off and send it to wine-staging that means something different to me than if I send it to wine-devel.
It should be taken to mean something like "I think that this is good enough to go into Wine". I don't think the meaning of Signed-off should change based on how the patch was submitted, particularly since patches can get into staging from various sources, including wine-devel.
If we want some mechanism to explicitly indicate that a patch is good enough for staging but not for main Wine, we should add a different header. I don't think that it should be the default assumption for anything that goes into staging, and I see no evidence that Michael meant it that way either.
On Wed, Mar 14, 2018 at 04:17:41PM +0100, Alexandre Julliard wrote:
Andrew Eikum aeikum@codeweavers.com writes:
On Wed, Mar 14, 2018 at 11:03:25AM +0100, Alexandre Julliard wrote:
If the patch is not changed, preserving the original sign-off is the right thing to do.
I thought Signed-off-by meant something like, "I agree to help debug this if something goes wrong." It seems wrong to make that statement to wine-devel on someone else's behalf. If I put a sign-off and send it to wine-staging that means something different to me than if I send it to wine-devel.
It should be taken to mean something like "I think that this is good enough to go into Wine". I don't think the meaning of Signed-off should change based on how the patch was submitted, particularly since patches can get into staging from various sources, including wine-devel.
Having a sign-off line on a patch floating around on the Internet shouldn't be taken as permission to submit the patch with my sign-off to wine-devel, in my opinion. Perhaps I've revised the patch elsewhere since then and I no longer think it's good enough for Wine.
In any case, the wiki currently disagrees with your interpretation:
"""Finally, your patch should include a Signed-off-by line. This line indicates that you accept responsibility for fixing any regressions caused by your patch."""
https://wiki.winehq.org/Submitting_Patches#The_commit_message
The wiki also suggests setting format.signOff, which would make sign-offs accidentally leak easily into patches uploaded to bugzilla, for example.
Andrew
2018-03-14 16:56 GMT+01:00 Andrew Eikum aeikum@codeweavers.com:
On Wed, Mar 14, 2018 at 04:17:41PM +0100, Alexandre Julliard wrote:
Andrew Eikum aeikum@codeweavers.com writes:
On Wed, Mar 14, 2018 at 11:03:25AM +0100, Alexandre Julliard wrote:
If the patch is not changed, preserving the original sign-off is the right thing to do.
I thought Signed-off-by meant something like, "I agree to help debug this if something goes wrong." It seems wrong to make that statement to wine-devel on someone else's behalf. If I put a sign-off and send it to wine-staging that means something different to me than if I send it to wine-devel.
It should be taken to mean something like "I think that this is good enough to go into Wine". I don't think the meaning of Signed-off should change based on how the patch was submitted, particularly since patches can get into staging from various sources, including wine-devel.
Having a sign-off line on a patch floating around on the Internet shouldn't be taken as permission to submit the patch with my sign-off to wine-devel, in my opinion. Perhaps I've revised the patch elsewhere since then and I no longer think it's good enough for Wine.
Personally I'm not signing off my own patches until I feel like they are ready for wine-devel i.e. a random patch attached to a bug most likely won't have my sign-off.
In any case, the wiki currently disagrees with your interpretation:
"""Finally, your patch should include a Signed-off-by line. This line indicates that you accept responsibility for fixing any regressions caused by your patch."""
https://wiki.winehq.org/Submitting_Patches#The_commit_message
The wiki also suggests setting format.signOff, which would make sign-offs accidentally leak easily into patches uploaded to bugzilla, for example.
Yeah, I don't like that. But that's the wiki and I think it should be updated if the general consensus differs.
Missatge de Andrew Eikum aeikum@codeweavers.com del dia dc., 14 de març 2018 a les 9:57:
On Wed, Mar 14, 2018 at 04:17:41PM +0100, Alexandre Julliard wrote:
Andrew Eikum aeikum@codeweavers.com writes:
On Wed, Mar 14, 2018 at 11:03:25AM +0100, Alexandre Julliard wrote:
If the patch is not changed, preserving the original sign-off is the right thing to do.
I thought Signed-off-by meant something like, "I agree to help debug this if something goes wrong." It seems wrong to make that statement to wine-devel on someone else's behalf. If I put a sign-off and send it to wine-staging that means something different to me than if I send it to wine-devel.
It should be taken to mean something like "I think that this is good enough to go into Wine". I don't think the meaning of Signed-off should change based on how the patch was submitted, particularly since patches can get into staging from various sources, including wine-devel.
Having a sign-off line on a patch floating around on the Internet shouldn't be taken as permission to submit the patch with my sign-off to wine-devel, in my opinion. Perhaps I've revised the patch elsewhere since then and I no longer think it's good enough for Wine.
In any case, the wiki currently disagrees with your interpretation:
"""Finally, your patch should include a Signed-off-by line. This line indicates that you accept responsibility for fixing any regressions caused by your patch."""
https://wiki.winehq.org/Submitting_Patches#The_commit_message
The wiki also suggests setting format.signOff, which would make sign-offs accidentally leak easily into patches uploaded to bugzilla, for example.
I wrote that wiki paragraph based on my understanding of the discussion at WineConf 2015. If I remember correctly, the primary concern was to know who to blame if a patch causes a regression. I didn't write the bit about Git's format.signOff option; I don't use it myself and I wouldn't recommend it.
My opinion is that if a patch was submitted to Staging with a sign-off, the sign-off should be kept, because Staging is an official part of Wine and its patches are intended to be upstreamed. However, if the patch was found on Bugzilla, GitHub, etc., the sign-off should not be kept without the original author's permission.
-Alex
Andrew Eikum aeikum@codeweavers.com writes:
On Wed, Mar 14, 2018 at 04:17:41PM +0100, Alexandre Julliard wrote:
Andrew Eikum aeikum@codeweavers.com writes:
On Wed, Mar 14, 2018 at 11:03:25AM +0100, Alexandre Julliard wrote:
If the patch is not changed, preserving the original sign-off is the right thing to do.
I thought Signed-off-by meant something like, "I agree to help debug this if something goes wrong." It seems wrong to make that statement to wine-devel on someone else's behalf. If I put a sign-off and send it to wine-staging that means something different to me than if I send it to wine-devel.
It should be taken to mean something like "I think that this is good enough to go into Wine". I don't think the meaning of Signed-off should change based on how the patch was submitted, particularly since patches can get into staging from various sources, including wine-devel.
Having a sign-off line on a patch floating around on the Internet shouldn't be taken as permission to submit the patch with my sign-off to wine-devel, in my opinion. Perhaps I've revised the patch elsewhere since then and I no longer think it's good enough for Wine.
In any case, the wiki currently disagrees with your interpretation:
"""Finally, your patch should include a Signed-off-by line. This line indicates that you accept responsibility for fixing any regressions caused by your patch."""
https://wiki.winehq.org/Submitting_Patches#The_commit_message
Who are you going to trust, me or the wiki? ;-)
That definition is probably too narrow. For instance, I add my signoff to the patches I commit to indicate that I think they are good enough for Wine. Of course in a sense I then accept responsibility for all regressions, but that's not the only meaning.
I think there's value in allowing people to submit through other channels but still indicate whether they think their patch is ready for Wine. Part of the motivation for Staging was to allow people to contribute without going through the hassle of wine-devel; declaring signoffs to be invalid unless you send to wine-devel yourself feels like a step backwards.
On 14 March 2018 at 17:57, Andrew Eikum aeikum@codeweavers.com wrote:
I thought Signed-off-by meant something like, "I agree to help debug this if something goes wrong." It seems wrong to make that statement to wine-devel on someone else's behalf. If I put a sign-off and send it to wine-staging that means something different to me than if I send it to wine-devel.
Well, I'd be inclined to agree with you for patches submitted to e.g. a different project. However, wine-devel and wine-staging are at least nominally the same project, and in theory patches are supposed to flow from wine-staging to wine-devel. Where it gets a little more complicated is that there may be some staging authors that don't see it that way; I don't think that's the case here though.
-
Alex Henrie -
Alexandre Julliard -
Andrew Eikum -
Dmitry Timoshkov -
Henri Verbeet -
Matteo Bruni -
Zebediah Figura