As it creates the same kind of objects, nsNSSIOLayer.cpp should use for consistency the same method as nsCertPicker.cpp to ensure they get properly destroyed. This means using the NSSCleanupAutoPtrClass macro around the elements that must be freed. Maybe adding a little comment to explain that "CERTCertNicknamesCleaner cnc(nicknames);" ensure that nicknames will be freed without having to explicitly free would be a good think. I oversighted it at first, and only undestood after wondering why CERT_DestroyCertList wasn't explicitly called in nsCertPicker.cpp. Isn't there a simple way to change the macro so that you don't have to do : CERTCertNicknames *nicknames = (getting new element ptr) CERTCertNicknamesCleaner cnc(nicknames); but simply CERTCertNicknamesAutoCleanPtr nicknames = (getting new element ptr) ? The "*" operator could be redefined to keep the pointer semantic. Also I think the exact same kind of code gets reused a lot, the patch for SignText has also very similar code to get a valid cert list, I wonder if it's not possible to define some functions to reduce the size. This is perhaps quite cosmetic, but I'm convinced nice looking, coherent code has less bugs, and on the long term, you end up spending more time correcting the bugs coming form inconcistencies than taking the time to write it clean first.
This bug was never closed, but the current nsNSSIOLayer.cpp code properly uses NSSCleanupAutoPtrClass where useful.