[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.documentfoundation.org/show_bug.cgi?id=71043 jan iversen changed: What|Removed |Added Whiteboard|target:4.3.0|ToBeReviewed -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.documentfoundation.org/show_bug.cgi?id=71043 Karthick Prasad Gunasekaran changed: What|Removed |Added Assignee|v...@libreoffice.org|libreoffice-b...@lists.free ||desktop.org -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 Commit Notification changed: What|Removed |Added Whiteboard|EasyHack DifficultyBeginner |EasyHack DifficultyBeginner |SkillCpp TopicCleanup |SkillCpp TopicCleanup ||target:4.3.0 --- Comment #15 from Commit Notification --- Jose Guilherme Vanz committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=f6141d884b7a89a856a34dc93991dbe01ded0b6b fdo#71043 - Use STACK lint tool to clean code The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 --- Comment #14 from Stephan Bergmann --- >From attachment 88771: > bug: anti-simplify > model: | > %68 = icmp eq %"class.configmgr::configuration_registry:: namespace>::RegistryKey"* %2, null, !dbg !5935 > --> false > stack: > - /home/vanz/git/mylibo/configmgr/source/configurationregistry.cxx:398:0 > ncore: 1 > core: > - > /home/vanz/git/mylibo/workdir/unxlngx6.pro/UnoApiHeadersTarget/udkapi/normal/com/sun/star/uno/XInterface.hdl:18:0 > - null pointer dereference For me, configmgr/source/configurationregistry.cxx:398 is > return new RegistryKey(*this, css::uno::makeAny(access_)); and workdir/unxlngx6.pro/UnoApiHeadersTarget/udkapi/normal/com/sun/star/uno/XInterface.hdl:18 is > class SAL_NO_VTABLE XInterface Are they any different for you? I'm not exactly sure what code this report is supposed to be about. It might be about the if statement in the implicit call to inline > template< class interface_type > > inline Reference< interface_type >::Reference( interface_type * pInterface ) > SAL_THROW(()) > { > _pInterface = castToXInterface(pInterface); > if (_pInterface) > _pInterface->acquire(); > } which is redundant in this particular case where pInterace is non-null "new RegistryKey(...)" and castToXInterface is an inline function that transfers non-null pointer to non-null pointer. But that would mean that STACK is unhelpfully too eager in reporting findings in inlined codes. -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 José Guilherme Vanz changed: What|Removed |Added Attachment #88526|0 |1 is obsolete|| --- Comment #13 from José Guilherme Vanz --- Created attachment 88771 --> https://bugs.freedesktop.org/attachment.cgi?id=88771&action=edit stack_result I executed STACK lint tool again. And finally! We can see some warnings in our codebase. -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 Stephan Bergmann changed: What|Removed |Added CC||sberg...@redhat.com --- Comment #12 from Stephan Bergmann --- (In reply to comment #5) > Now, what to do in this particular case then is a question of taste, more or > less. > > Personally I think that if a memory allocation (of a small amount of memory) > fails at this place in the code, which as far as I know is invoked mainly > (only?) very early when LO is starting, something is very wrong and there is > no point in even trying to recover gracefully from the memory allocation > failure. So let __osl_createPipeImpl() be as is and remove the pointless > checks for NULL return after the calls to it. > > But I assume there is an opposite opinion, too, that each and every memory > allocation should be checked and the code should do its utmost to fail > gracefully... In that case, the check for memory allocation failure should > be moved inside __osl_createPipeImpl() right after the call to calloc(), and > in case of failure, NULL should be returned immediately. Hopefully then the > return value of __osl_createPipeImpl() is checked in each case. In this case, where both callers of __osl_createPipeImpl already use "return NULL" to indicate error, I see no disadvantage in cleanly reporting calloc failure instead of relying on SIGSEGV doing the right thing. -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 --- Comment #11 from José Guilherme Vanz --- Yeap, last night I executed autogen again and compiled with clang again. Now, I think is working well. But I'm still investigating why STACK is not working very well in our codebase. -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 --- Comment #10 from Tor Lillqvist --- I, and several other people, compile LibreOffice using Clang all the time, both on Linux (where Clang so far is not mainstream, I guess) and on OS X (for OS X itself and cross-compiling for iOS) (where Clang *is* mainstream, no gcc available any more at all). This has been done for over a year at least. So it isn't as if it would be anything new to compile LibrOffice using Clang. The configure.ac has logic to decide whether to use the -fno-enforce-eh-specs option or not, and so far it has worked. So either your version of Clang is in some way then odd and the configure.ac tests break down, or you are telling it to use Clang in some wrong way. What does your autogen.input look like? For me, to use Clang (on Linux), I simply add CC=clang and CXX=clang++ lines to autogen.input. -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 --- Comment #8 from José Guilherme Vanz --- I was looking for the reason the STACK does not works well in our dabatase. Well, STACK use clang compiler to does its job. So, if we compile libo with clang we can see this error message: clang: error: unknown argument: '-fno-enforce-eh-specs' How can I solve the problem? How can I remove the argument when compile if clang? -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 --- Comment #7 from José Guilherme Vanz --- Yeap, analyzing the file more calm I saw what you are saying. Actually, I was a little exciting with the stack, so I posted right after ran the tool. =P About external source I don't know if a good ideia to try fix it. Maybe, send a report for their developers... It's a possibility. And about LLVM ERROR, I have already send a e-mail for stack maintainers. -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 --- Comment #6 from Tor Lillqvist --- Reading more of the report, it seems truncated and/or a bit useless? Everything else in it is related to external (bundled) sources, not sure if we want to bother with fixing those? And then there are thousands of repeated "LLVM ERROR: Bad DataLayout ctor used. Tool did not specify a DataLayout to use?" lines at the end. Is our C++ code somehow too pathological for this tool? (That would not be surprising of course... this codebase has been breaking toolchains for a quarter of a century I think;) Anyway, yay, this is a great start, but for the majority of our codebase it seems that the tool fails, so that needs to be investigated in detail. (By looking at what happens when just one single source file is run through the tool, and either sending a bug report to the STACK maintainers and hoping they can fix it, or figure out what we need to to do work around the problem.) -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 --- Comment #5 from Tor Lillqvist --- Whee, yes, the results looks indeed quite interesting, even if it takes a (short) while to understand what they are trying to say. Just look at the first thing reported, in sal/osl/unx/pipe.c: in __osl_createPipeImpl(): pPipeImpl = (oslPipe)calloc(1, sizeof(struct oslPipeImpl)); pPipeImpl->m_nRefCount =1; ... return pPipeImpl; Note that there is no check whether calloc() fails. The code immediately dereferences the returned pointer (which might in theory be NULL, in which case the dereferencing will cause a crash of course). The pointer is returned as the function value. So let's look at the reported call site of __osl_createPipeImpl(): pAcceptedPipe= __osl_createPipeImpl(); OSL_ASSERT(pAcceptedPipe); if(pAcceptedPipe==NULL) { close(s); return NULL; } Ha! The checks whether pAcceptedPipe is NULL are totally pointless as we have already dereferenced it inside __osl_createPipeImpl(), so it can't be NULL, and the whole if statement will be optimised away by a modern compiler. Now, what to do in this particular case then is a question of taste, more or less. Personally I think that if a memory allocation (of a small amount of memory) fails at this place in the code, which as far as I know is invoked mainly (only?) very early when LO is starting, something is very wrong and there is no point in even trying to recover gracefully from the memory allocation failure. So let __osl_createPipeImpl() be as is and remove the pointless checks for NULL return after the calls to it. But I assume there is an opposite opinion, too, that each and every memory allocation should be checked and the code should do its utmost to fail gracefully... In that case, the check for memory allocation failure should be moved inside __osl_createPipeImpl() right after the call to calloc(), and in case of failure, NULL should be returned immediately. Hopefully then the return value of __osl_createPipeImpl() is checked in each case. -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 --- Comment #4 from Tor Lillqvist --- Great! But give people a chance to look in detail at the results first. Actually, the developer mailing list would be best for discussion about this, I think? -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 --- Comment #3 from José Guilherme Vanz --- Can we start change the code where stack warned us? -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 --- Comment #2 from José Guilherme Vanz --- Created attachment 88526 --> https://bugs.freedesktop.org/attachment.cgi?id=88526&action=edit stack_result This is the results after ran stack on libreoffice code. -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 --- Comment #1 from José Guilherme Vanz --- I will take a look on this. :-) -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 José Guilherme Vanz changed: What|Removed |Added Assignee|libreoffice-b...@lists.free |v...@libreoffice.org |desktop.org | -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Bug 71043] Use STACK lint tool to clean code ...
https://bugs.freedesktop.org/show_bug.cgi?id=71043 Michael Meeks changed: What|Removed |Added Whiteboard||EasyHack DifficultyBeginner ||SkillCpp TopicCleanup -- You are receiving this mail because: You are on the CC list for the bug. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice