ELF Dissector in kdereview

[Volker Krause]

Thanks for the feedback :)

On Sunday, 29 September 2019 12:51:03 CEST Albert Astals Cid wrote:
> El dissabte, 28 de setembre de 2019, a les 13:01:11 CEST, Volker Krause va 
escriure:
> > Hi,
> > 
> > ELF Dissector has been moved to kdereview for the usual review process.
> 
> It doesn't build for me, i need
> 
> -#include <capstone.h>
> +#include <capstone/capstone.h>
> 
> Because my include is in
>   /usr/include/capstone/capstone.h
> and
>   pkg-config --cflags capstone
> returns empty.

Fixed, their pkgconfig file differs based on version and used build system, we 
can handle both now.

> clang-tidy says
>   src/ui/mainwindow.cpp:120:29: error: std::move of the const variable
> 'fileName' has no effect; remove std::move() or make the variable non-const
> [performance-move-const-arg,-warnings-as-errors] m_currentFileName =
> std::move(fileName);
> So
> -        const auto fileName =
> settings.value(QStringLiteral("Recent/PreviousFile")).toString(); +       
> auto fileName =
> settings.value(QStringLiteral("Recent/PreviousFile")).toString(); i guess?
> 
> 
>   src/lib/checks/ldbenchmark.cpp:161:20: error: Value stored to 'file'
> during its initialization is never read
> [clang-analyzer-deadcode.DeadStores,-warnings-as-errors] const auto file =
> m_fileSet->file(m_results.size() - 1 - i);

Both fixed.

>   src/lib/printers/relocationprinter.cpp:416:8: error: Excessive padding in
> 'struct RelocTypeRepository' (8 padding bytes, where 0 is optimal). Optimal
> fields order:
>       typeInfos,
>       machine,
>       typeInfosSize,
>     consider reordering the fields or adding explicit padding members
> [clang-analyzer-optin.performance.Padding,-warnings-as-errors] (Annoying i
> know, but isn't elf-dissector a bit about this padding things too?)

Fixed. Kinda ironic, considering ELF Dissector was (to my knowledge) the first 
tool out there that could find such memory layout issues for more complex C++ 
types too (apart from virtual inheritance, the memory layout for that is ... 
interesting), way before clang got that warning (which of course is the much 
better place for this) :)

> Also you have a few deprecated Qt calls.

Seems mostly from the treemap copy, updated to the latest version.

Regards,
Volker
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20190930/36c27b14/attachment.sig>