Remove cast in PR_REMOVE_LINK() invocation in tls13_CipherSpecRelease() for consistency

RESOLVED FIXED in 3.26

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ttaubert, Assigned: ttaubert)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
> void
> tls13_CipherSpecRelease(ssl3CipherSpec *spec)
> {
>     PORT_Assert(spec->refCt > 0);
>     --spec->refCt;
>     SSL_TRC(50, ("%d: TLS 1.3: Decrement ref ct for spec %d. new ct = %d",
>                  SSL_GETPID(), spec, spec->refCt));
>     if (!spec->refCt) {
>         SSL_TRC(50, ("%d: TLS 1.3: Freeing spec %d",
>                      SSL_GETPID(), spec));
>         PR_REMOVE_LINK((PRCList *)spec);

Should be PR_REMOVE_LINK(&spec->link);

>         ssl3_DestroyCipherSpec(spec, PR_TRUE);
>         PORT_Free(spec);
>     }
> }
(Assignee)

Comment 1

2 years ago
Created attachment 8762831 [details] [diff] [review]
0001-Bug-1280199-Wrong-cast-in-PR_REMOVE_LINK-invocation-.patch
Attachment #8762831 - Flags: review?(martin.thomson)
Attachment #8762831 - Flags: review?(ekr)
(Assignee)

Comment 2

2 years ago
ekr just noted that the code isn't actually wrong as long as "link" is the first field in the ssl3CipherSpec struct. I think it would be good to take the patch anyway, if only to clarify.

Comment 3

2 years ago
I believe that this is actually correct because it's the first item in the struct:

typedef struct {
    PRCList link;
...

};

if you have ssl3CipherSpec *s, then (PRCList *)s == &s->link, per C standard 6.7.2.1.13.


However, r=me for this patch for consistency.

Updated

2 years ago
Attachment #8762831 - Flags: review?(martin.thomson) → review+
(Assignee)

Comment 4

2 years ago
https://hg.mozilla.org/projects/nss/rev/ecdc269760b0
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.26
(Assignee)

Updated

2 years ago
Attachment #8762831 - Flags: review?(ekr)
(Assignee)

Updated

2 years ago
Summary: Wrong cast in PR_REMOVE_LINK() invocation in tls13_CipherSpecRelease() → Remove cast in PR_REMOVE_LINK() invocation in tls13_CipherSpecRelease() for consistency
You need to log in before you can comment on or make changes to this bug.