Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-18 Thread Amine Khaldi
Not really, it's up to you too. This is essentially a style-only change. Alexandre generally frowns on these, but reluctantly accepts them if the existing style is horrible, or if you're actively involved in the code being modified. Neither appears to be true here. This is a helpful hint to a

Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-18 Thread Amine Khaldi
It makes the code use one less variable. I don't see how is this dubious, but I understand that it's trivial. Yes, at the cost of potentially less readability, or, depending on the compile settings, a little more difficulty in checking whether the function succeeded. I don't see that the

Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-18 Thread Amine Khaldi
Hi Amine, Hi Juan, this patch has no functional benefit. For example, -HRESULT hr; TRACE("\n"); -hr = SMTPTransport_ParseResponse(This, pBuffer,&response); -if (FAILED(hr)) +if (FAILED(SMTPTransport_ParseResponse(This, pBuffer,&response))) This has the dubious ben

Re: inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-18 Thread Amine Khaldi
Hey Chip, I think he's using Clang. I've seen him on the LLVM bugzilla. He's waiting for someone (e.g. me) to fix the bugs that prevent Wine from being compiled with Clang. That's right. BTW, if and when you find a bug using Clang, be sure to put "(Clang/LLVM)" in the title--or at least menti

Re: inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-18 Thread Amine Khaldi
Out of curiosity, what tool are you using to find these bugs? Clang. WBR, Amine.

Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-16 Thread Henri Verbeet
2009/12/16 James Hawkins : > Also, you should never pass a function to a macro.  What if FAILED > were defined as: > > FAILED(x) (x & MASK1) | (x & MASK2) > > ? > Then the macro is broken, although admittedly most macros probably are.

Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-16 Thread James Hawkins
On Wed, Dec 16, 2009 at 11:09 AM, Juan Lang wrote: > Hi Amine, > > this patch has no functional benefit.  For example, > -    HRESULT hr; > >     TRACE("\n"); > > -    hr = SMTPTransport_ParseResponse(This, pBuffer, &response); > -    if (FAILED(hr)) > +    if (FAILED(SMTPTransport_ParseResponse(T

Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-16 Thread Juan Lang
> Well, that's entirely up to you guys ;) Not really, it's up to you too. This is essentially a style-only change. Alexandre generally frowns on these, but reluctantly accepts them if the existing style is horrible, or if you're actively involved in the code being modified. Neither appears to b

Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-16 Thread Juan Lang
> It makes the code use one less variable. I don't see how is this dubious, > but I understand that it's trivial. Yes, at the cost of potentially less readability, or, depending on the compile settings, a little more difficulty in checking whether the function succeeded. I don't see that the bene

Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-16 Thread Juan Lang
Hi Amine, this patch has no functional benefit. For example, -HRESULT hr; TRACE("\n"); -hr = SMTPTransport_ParseResponse(This, pBuffer, &response); -if (FAILED(hr)) +if (FAILED(SMTPTransport_ParseResponse(This, pBuffer, &response))) This has the dubious benefit of removing

Re: inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-16 Thread Charles Davis
Austin English wrote: > On Wed, Dec 16, 2009 at 12:28 PM, Amine Khaldi wrote: >> WBR, >> Amine Khaldi. > > Out of curiosity, what tool are you using to find these bugs? > I think he's using Clang. I've seen him on the LLVM bugzilla. He's waiting for someone (e.g. me) to fix the bugs that prevent

Re: inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-16 Thread Austin English
On Wed, Dec 16, 2009 at 12:28 PM, Amine Khaldi wrote: > WBR, > Amine Khaldi. Out of curiosity, what tool are you using to find these bugs? -- -Austin

Re: inetcomm/smtptransport.c : Remove some unneeded variables and assigns.

2009-12-16 Thread Austin English
On Wed, Dec 16, 2009 at 12:28 PM, Amine Khaldi wrote: > WBR, > Amine Khaldi. Out of curiosity, what tool are you using to find these bugs? -- -Austin