>> + certs = CERT_CreateSubjectCertList (NULL, handle,
> &cert->derSubject,
>
> Doesn't this need a return value test? AFAICT,
> CERT_CreateSubjectCertList might return NULL, and CERTLIST_HEAD (certs)
> will unconditionally look up a member? There's a second instance of this
> pattern in print_trusted_certificates().
Agreed. I can expand the code to make it check for NULL.
> However, since the postinst only calls nss-database-pem-exporter from
> inside import_nss_ca_certs(), the "set -e" won't have any effect there,
> so I think this is OK in practice. I'd normally ask for more explicit
> error handling (or at least comments in the postinst) but since this
> migration code will only exist in this SRU, I think it's OK to leave it
> as-is.
>
>> + if dpkg --compare-versions "$2" lt-nl 2.2.3-3ubuntu0.2; then
>
> Doesn't this now need bumping to 0.4? The current version in focal-
> updates is 2.2.3-3ubuntu0.3. Otherwise I think the upgrade path won't
> activate for anyone already on 2.2.3-3ubuntu0.2 or 2.2.3-3ubuntu0.3?
Yes, this one slipped past my radar. There were two more uploads since
Marco posted his first MP, and although he did rebase it against the
latest sssd on Focal, we forgot about this check.
How should I proceed here? Should I just upload the new package with
the same version, since it wasn't accepted yet?
On Thursday, February 18 2021, Robie Basak wrote:
>> + certs = CERT_CreateSubj ectCertList (NULL, handle, ectCertList might return NULL, and CERTLIST_HEAD (certs) certificates( ).
> &cert->derSubject,
>
> Doesn't this need a return value test? AFAICT,
> CERT_CreateSubj
> will unconditionally look up a member? There's a second instance of this
> pattern in print_trusted_
Agreed. I can expand the code to make it check for NULL.
> However, since the postinst only calls nss-database- pem-exporter from nss_ca_ certs() , the "set -e" won't have any effect there,
> inside import_
> so I think this is OK in practice. I'd normally ask for more explicit
> error handling (or at least comments in the postinst) but since this
> migration code will only exist in this SRU, I think it's OK to leave it
> as-is.
>
>> + if dpkg --compare-versions "$2" lt-nl 2.2.3-3ubuntu0.2; then
>
> Doesn't this now need bumping to 0.4? The current version in focal-
> updates is 2.2.3-3ubuntu0.3. Otherwise I think the upgrade path won't
> activate for anyone already on 2.2.3-3ubuntu0.2 or 2.2.3-3ubuntu0.3?
Yes, this one slipped past my radar. There were two more uploads since
Marco posted his first MP, and although he did rebase it against the
latest sssd on Focal, we forgot about this check.
How should I proceed here? Should I just upload the new package with
the same version, since it wasn't accepted yet?
Thanks,
--
Sergio
GPG key ID: E92F D0B3 6B14 F1F4 D8E0 EB2F 106D A1C8 C3CB BF14