Status

defect
P3
normal
ASSIGNED
20 years ago
13 years ago

People

(Reporter: erik, Assigned: wtc)

Tracking

other
Future

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

The PR_FREEIF macro is dangerous. If it is followed by "else", it will give an
unexpected result:

  if (foo)
    PR_FREEIF(ptr);
  else
    blah();

I suggest putting PR_BEGIN_MACRO and PR_END_MACRO around the relevant macros.
The nested if-else problem.

The fix you suggested is the right one.
Status: NEW → ASSIGNED
Target Milestone: --- → 4.0.1
Target Milestone: 4.0.1 → 4.1
In fact the sample code:
  if (foo)
    PR_FREEIF(ptr);
  else
    blah();
won't even compile because of the way
PR_DELETE is defined.  (PR_FREEIF uses
PR_DELETE.)  So PR_DELETE also needs
to be fixed.

I'm going to attach a patch that changes
PR_DELETE and PR_FREEIF to use PR_BEGIN_MACRO
and PR_END_MACRO.
The patch is checked in on the main trunk.
/cvsroot/mozilla/nsprpub/pr/include/prmem.h, revision 3.7

Added a new test freeif.c which contains Erik's
sample code.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
It was pointed out to me that some code
is depending on the current broken definition
of PR_DELETE and PR_FREEIF.  For example,
some files contain instances of PR_FREEIF(ptr)
that don't end with ';'.

I'm going to defer this change to the next
major release, NSPR 5.0, because it breaks
backward compatibility at source level.

Backed out the checkin.
/cvsroot/mozilla/nsprpub/pr/include/prmem.h, revision 3.8

The test freeif.c is removed from the makefile and test
harness.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 4.1 → Future
Status: REOPENED → ASSIGNED
Are PR_DELETE and PR_FREEIF still broken?
As comment #5 seems to indicate that his bug is dependant on these...
QA Contact: wtchang → nspr
You need to log in before you can comment on or make changes to this bug.