<html>
<head>
<base href="https://bugs.freedesktop.org/">
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - make it possible to extract digital signature data (also in pdfsig)"
href="https://bugs.freedesktop.org/show_bug.cgi?id=99271#c11">Comment # 11</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - make it possible to extract digital signature data (also in pdfsig)"
href="https://bugs.freedesktop.org/show_bug.cgi?id=99271">bug 99271</a>
from <span class="vcard"><a class="email" href="mailto:aacid@kde.org" title="Albert Astals Cid <aacid@kde.org>"> <span class="fn">Albert Astals Cid</span></a>
</span></b>
<pre><span class="quote">> Consider using a std::unique_ptr here, to make it clear that the method does
> not retain ownership.</span >
No std::unique_ptr for now please, we don't really use them in core yet.
<span class="quote">> {
> - if (!CMSSignerInfo)
> - return NULL;
> + if (CMSSignerInfo == nullptr)
> + return nullptr;
>
> This is purely cosmetic. Not sure if that is allowed.</span >
Cosmetic changes are evil because make reviewing harder for no reason, one has
to carefully look at the change to see if the logic broke or not, please
refrain for any cosmetic change
<span class="quote">>
> +const char * SignatureHandler::getSignerSubjectDN()
> +{
> + if (CMSSignerInfo == nullptr)
> + return nullptr;
> +
>
> if (!CMSSignerInfo) seems to be the prefered pattern in poppler. The if
> (... == nullptr)
> way appears various times in this patch. Not sure whether it is okay to
> keep it.</span >
Yes, don't do == nullptr</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>