Re: aclui: Remove some unneeded header inclusions. (IWYU)
On 06/09/2013 16:28, Alexandre Julliard wrote: Amine Khaldi amine.kha...@reactos.org writes: @@ -18,15 +18,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ -#include config.h - -#include stdarg.h - #include initguid.h -#include windef.h -#include winbase.h -#include winuser.h -#include winnt.h #include aclui.h That's not useful. The headers will be included anyway. Indeed. Are there some benefits of including them more than once here ? Regards, Amine.
Re: [PATCH] activeds: Exclude unused headers.
On 12/03/2013 11:27, Alexandre Julliard wrote: Amine Khaldi amine.kha...@reactos.org writes: @@ -28,13 +28,10 @@ #include windef.h #include winbase.h -#include winuser.h #include objbase.h #include iads.h -#include adshlp.h -#include wine/unicode.h #include wine/debug.h It's not worth the trouble, particularly since the file ends up including windows.h anyway. In this case ending up with windows.h is not needed, so if we: #define WIN32_NO_STATUS #define _INC_WINDOWS #define COM_NO_WINDOWS_H That prevents it. Should I send a second patch with these defines ? Regards, Amine.
Coverity Scans Revived
Hello folks, Recently we (myself and Urias from Haiku) have been working on getting the coverity scans up and running again. I'm happy to announce that we succeeded ! :) I submitted a scan and the results are up online. I'll submit further scans when necessary, like it's already done with clang analyzer scans. Have fun ! Regards, Amine.
Static analysis scan
Hello folks, I've set up a static analysis scan using clang static analyzer. The results are available at http://austinenglish.com/logs/clang_analyzer/index.html thanks to Austin for the web space. Please feel free to fix the defects. I'll run future scans when the defects get fixed progressively. Have fun ;) With best regards, Amine.
Re: hlink/hlink_main.c : Remove an unneeded assignment.
Hi Amine, Hi Jacek, r = register_clsid(CLSID_StdHlink); if (SUCCEEDED(r)) -r = register_clsid(CLSID_StdHlinkBrowseContext); +register_clsid(CLSID_StdHlinkBrowseContext); return S_OK; In this case the correct fix is to return r. Please be more careful then doing janitorial work. There are more places in your patches that seem questionable. You should make sure that we want to ignore an error instead of handling it (and usually we don't) before deleting such assignments. Otherwise it's just hiding bugs. Hmm.. I missed this point, sorry. Thanks, Amine.
Re: inetcomm/smtptransport.c : Remove some unneeded variables and assigns.
Out of curiosity, what tool are you using to find these bugs? Clang. WBR, Amine.
Re: inetcomm/smtptransport.c : Remove some unneeded variables and assigns.
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 mention that you used Clang. Noted. WBR, Amine.
Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.
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 benefit of removing a local variable that was used before. I can't fathom why this is a good thing. It makes the code use one less variable. I don't see how is this dubious, but I understand that it's trivial. WBR, Amine.
Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.
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 benefits outweigh the costs. Well, that's entirely up to you guys ;) Amine.
Re: [PATCH] inetcomm/smtptransport.c : Fix a typo ?
Hi Amine, Hi Juan, Um, it's called a prototype, and it's needed to compile smtptransport.c. Have you tested this patch at all? I knew it was a prototype, I just thought it was left over, as it wasn't at the top of the file, and I saw no other prototype in that file. As for testing, I was eventually going to re-compile, so I guess I'd figure this out later if you didn't mention it. Amine.
Re: [PATCH] inetcomm/smtptransport.c : Remove some unneeded variables and assigns.
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 avoid you killing your AJ-rank unnecessarily: don't do that, your patches are less likely to get accepted in the future. Thank you for the hint, I wasn't aware of this (I'm sort of a new comer).. noted. AJ-rank, hein ? :) Amine.