Closed Bug 1588015 Opened 5 months ago Closed 4 months ago

Provide alternative PR_ASSERT that "uses" inputs always

Categories

(NSPR :: NSPR, enhancement, P2)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mt, Assigned: marcus.apb)

References

Details

Attachments

(1 file)

Technically, this isn't a problem, but it creates some odd cases. Say that I wanted to assert the return value of a function, like this bit in certdb.c:

#ifdef DEBUG
    {
        PRStatus prstat = PZ_Unlock(certTempPermLock);
        PORT_Assert(prstat == PR_SUCCESS); // PORT_Assert is defined as PR_ASSERT
    }
#else
    (void)PZ_Unlock(certTempPermLock);
#endif

This is pretty ugly, but you get unused variable warnings if you simplify to:

    PRStatus prstat = PZ_Unlock(certTempPermLock);
    PORT_Assert(prstat == PR_SUCCESS);

You could change the definition of PR_ASSERT to this:

#define PR_ASSERT(expr) ((void)(0 && (expr)))

Which optimizes out the expression easily (-01 is enough in gdb), but we have code that does this:

#ifdef DEBUG
    int x = 0;
#endif
   // ... 
#ifdef DEBUG
    x = something;
#endif
   // ...
    PR_ASSERT(x);

Which fails even harder because x is undefined on the unguarded PR_ASSERT invocation. That code should probably just guard the assertion call as well, but there's plenty of code built this way already and we can't break that code.

So I'm suggesting that we add a macro that "uses" its argument. I'm suggesting it here rather than just for NSS, because that has broader applicability, but I will also open an NSS to move to this model or to use this model of assertion (PORT_Assert is also subject to the same constraints as PR_ASSERT, so we have the same problem there).

Assignee: nobody → marcus.apb
Priority: -- → P2
Blocks: 1589073
Status: NEW → ASSIGNED
Blocks: 1591887
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 4.24
You need to log in before you can comment on or make changes to this bug.