Re: iphlpapi: Let C look like C.
On Feb 6, 2013 11:13 PM, "Michael Stefaniuc" <mstefani(a)redhat.de> wrote:
--- dlls/iphlpapi/ipstats.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/dlls/iphlpapi/ipstats.c b/dlls/iphlpapi/ipstats.c index f8191ba..966d24e 100644 --- a/dlls/iphlpapi/ipstats.c +++ b/dlls/iphlpapi/ipstats.c @@ -693,7 +693,7 @@ DWORD WINAPI GetIcmpStatisticsEx(PMIB_ICMP_EX stats,
DWORD family)
}
ret = GetIcmpStatistics(&ipv4stats); - if SUCCEEDED(ret) + if (SUCCEEDED(ret)) { stats->icmpInStats.dwMsgs = ipv4stats.stats.icmpInStats.dwMsgs; stats->icmpInStats.dwErrors =
ipv4stats.stats.icmpInStats.dwErrors;
-- 1.7.6.5
Minor nitpick, "Make C look like C" would be a more proper title, imho.
On 02/06/2013 11:16 PM, Austin English wrote:
On Feb 6, 2013 11:13 PM, "Michael Stefaniuc" <mstefani(a)redhat.de <mailto:mstefani(a)redhat.de>> wrote:
--- dlls/iphlpapi/ipstats.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/dlls/iphlpapi/ipstats.c b/dlls/iphlpapi/ipstats.c index f8191ba..966d24e 100644 --- a/dlls/iphlpapi/ipstats.c +++ b/dlls/iphlpapi/ipstats.c @@ -693,7 +693,7 @@ DWORD WINAPI GetIcmpStatisticsEx(PMIB_ICMP_EX
stats, DWORD family)
}
ret = GetIcmpStatistics(&ipv4stats); - if SUCCEEDED(ret) + if (SUCCEEDED(ret)) { stats->icmpInStats.dwMsgs = ipv4stats.stats.icmpInStats.dwMsgs; stats->icmpInStats.dwErrors =
ipv4stats.stats.icmpInStats.dwErrors;
-- 1.7.6.5
Minor nitpick, "Make C look like C" would be a more proper title, imho. Yeah, but then I would describe what I'm doing and that is a big NO NO as it is obvious from the patch. The intend is to let the Wine C code feel like a first class C citizen ;-P
bye michael
Hi Michael, this isn't actually a problem with your patch, just something I spotted: On Wed, Feb 6, 2013 at 3:15 PM, Michael Stefaniuc <mstefani(a)redhat.com>wrote:
On 02/06/2013 11:16 PM, Austin English wrote:
On Feb 6, 2013 11:13 PM, "Michael Stefaniuc" <mstefani(a)redhat.de <mailto:mstefani(a)redhat.de>> wrote:
--- dlls/iphlpapi/ipstats.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/dlls/iphlpapi/ipstats.c b/dlls/iphlpapi/ipstats.c index f8191ba..966d24e 100644 --- a/dlls/iphlpapi/ipstats.c +++ b/dlls/iphlpapi/ipstats.c @@ -693,7 +693,7 @@ DWORD WINAPI GetIcmpStatisticsEx(PMIB_ICMP_EX
stats, DWORD family)
}
ret = GetIcmpStatistics(&ipv4stats); - if SUCCEEDED(ret) + if (SUCCEEDED(ret))
This code is incorrect. All paths that set ret yield a non-negative value for ret, so SUCCEEDED(ret) is always 1. Using SUCCEEDED/FAILED on something other than an HRESULT is asking for trouble. The correct thing to do is if (!ret). --Juan
Thanks Juan, On 02/07/2013 12:54 AM, Juan Lang wrote:
Hi Michael, this isn't actually a problem with your patch, just something I spotted:
On Wed, Feb 6, 2013 at 3:15 PM, Michael Stefaniuc <mstefani(a)redhat.com <mailto:mstefani(a)redhat.com>> wrote:
On 02/06/2013 11:16 PM, Austin English wrote: > On Feb 6, 2013 11:13 PM, "Michael Stefaniuc" <mstefani(a)redhat.de <mailto:mstefani(a)redhat.de> > <mailto:mstefani(a)redhat.de <mailto:mstefani(a)redhat.de>>> wrote: >> >> --- >> dlls/iphlpapi/ipstats.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/dlls/iphlpapi/ipstats.c b/dlls/iphlpapi/ipstats.c >> index f8191ba..966d24e 100644 >> --- a/dlls/iphlpapi/ipstats.c >> +++ b/dlls/iphlpapi/ipstats.c >> @@ -693,7 +693,7 @@ DWORD WINAPI GetIcmpStatisticsEx(PMIB_ICMP_EX > stats, DWORD family) >> } >> >> ret = GetIcmpStatistics(&ipv4stats); >> - if SUCCEEDED(ret) >> + if (SUCCEEDED(ret))
This code is incorrect. All paths that set ret yield a non-negative value for ret, so SUCCEEDED(ret) is always 1. Using SUCCEEDED/FAILED on something other than an HRESULT is asking for trouble. The correct thing to do is if (!ret). can you please submit the patch so you get the commit glory?
thanks bye michael
participants (3)
-
Austin English -
Juan Lang -
Michael Stefaniuc