PSM : consistency in use of cleaners between nsNSSIOLayer.cpp and nsCertPicker.cpp

RESOLVED FIXED

Status

Core Graveyard
Security: UI
--
minor
RESOLVED FIXED
14 years ago
11 months ago

People

(Reporter: Jean-Marc Desperrier, Unassigned)

Tracking

Other Branch

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [kerh-cuz])

(Reporter)

Description

14 years ago
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.

Updated

14 years ago
Summary: PSM : consistency in use of cleaners between nsNSSIOLayer.cpp and nsCertPicker.cpp → PSM : consistency in use of cleaners between nsNSSIOLayer.cpp and nsCertPicker.cpp

Updated

13 years ago
Assignee: kaie → nobody

Updated

13 years ago
Component: Security: UI → Security: UI
Product: PSM → Core

Updated

12 years ago
Whiteboard: [kerh-cuz]
QA Contact: bmartin → ui
(Reporter)

Comment 1

6 years ago
This bug was never closed, but the current nsNSSIOLayer.cpp code properly uses NSSCleanupAutoPtrClass where useful.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 months ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.