Last Comment Bug 220357 - PSM : consistency in use of cleaners between nsNSSIOLayer.cpp and nsCertPicker.cpp
: PSM : consistency in use of cleaners between nsNSSIOLayer.cpp and nsCertPicke...
Status: RESOLVED FIXED
[kerh-cuz]
:
Product: Core
Classification: Components
Component: Security: UI (show other bugs)
: Other Branch
: All All
: -- minor (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-09-26 05:34 PDT by Jean-Marc Desperrier
Modified: 2011-11-28 09:10 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Jean-Marc Desperrier 2003-09-26 05:34:09 PDT
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.
Comment 1 Jean-Marc Desperrier 2011-11-28 09:10:57 PST
This bug was never closed, but the current nsNSSIOLayer.cpp code properly uses NSSCleanupAutoPtrClass where useful.

Note You need to log in before you can comment on or make changes to this bug.