104102 - Let's add a new template auto pointer to allow for elegant code without goto

Status: VERIFIED WONTFIX

(Core Graveyard :: Security: UI, defect, P2)

Description

[Kai Engert [:KaiE:]]

Currently, PSM uses a lot of goto statements. While goto might be fine for a C
library, as NSS itself, it causes several problems for C++ code. For example,
you can't put your declaration statements anywhere you want to, because a goto
must not cross init of variables.

At least I don't want to use goto in the new C++ code I'm writing.

To allow for this, I'm suggesting the following new class. It is currently a
very simple class, not sure how much we want to drive this.

It does not use reference counting, it is currently designed to be used in one
function only, and not to be passed around.

The usage is: You construct it, as the template argument, with the type of the
NSS struct that you wish to store in the auto pointer. The constructor takes the
pointer to the NSS data object, and the name of the function that should be
called for cleaning up when the scope is left.

I wonder how much additional work must be put into this to make it compile on
all of our platforms.

template <class T>
class nsNSSAutoCPtr
{
public:

  typedef void (*destroyer)(T* t);

  nsNSSAutoCPtr<T>( T* t, destroyer d)
  :data(t), data_destroyer(d)
  {
  }
  
  ~nsNSSAutoCPtr<T>()
  {
    if (data && data_destroyer)
    {
      (*data_destroyer)(data);
    }
  }
  
  operator T*()
  {
    return data;
  }
  
  T& operator*()
  {
    return *data;
  }

  T* operator->()
  {
    return data;
  }
  
  PRBool operator!()
  {
    return !data;
  }
  
  void setPtr( T *t)
  {
    data = t;
  }
  
  T* ptr()
  {
    return data;
  }

private:

  T *data;
  destroyer data_destroyer;

};

Comment 1

[Kai Engert [:KaiE:]]

-> kaie

Here is an example for how to use the class:

  nsNSSAutoCPtr<CERTCertNicknames> nicknames(
    CERT_NicknameStringsFromCertList(certList,
                                     NICKNAME_EXPIRED_STRING,
                                     NICKNAME_NOT_YET_VALID_STRING),
    CERT_FreeNicknames
  );

Comment 2

[Kai Engert [:KaiE:]]

By the way, by overloading the operators, the existing code doesn't need to be
changed. The class can just be used whereever the raw pointer is used. Operator
-> works, too.

The code already compiles on VC++ and gcc.

Comment 3

[Stephane Saux]

cc rangansen
What's the impact on overhead, code bload. Can the operators be inlined?

Comment 4

[Kai Engert [:KaiE:]]

It's completely inlined. There is only this header file, which defines all
methods inline. There is no .cpp file, I don't think one will be necessary.

Comment 5

[Kai Engert [:KaiE:]]

The feature suggested in this bug is no longer needed, marking WONTFIX.

Some time ago I landed a helper class, implemented in
security/manager/ssl/src/nsNSSCleaner.h.

It is simpler and does require use of templates, it simulates by using #define
and ## preprocessor operator.

If you are interested to avoid goto, search security code for
NSSCleanupAutoPtrClass to see examples how it can be used.

Comment 6

[Kai Engert [:KaiE:]]

It is simpler and does require use of templates
                      ^
                     not

Comment 7

[John Unruh]

Verified per kaie's comments.