nats-io / nats.c

A C client for NATS
Apache License 2.0
390 stars 137 forks source link

Possible error in stan/pub.c #668

Closed NostraMagister closed 1 year ago

NostraMagister commented 1 year ago

In an attempt to better understand i stumbled on this:

source: https://github.com/nats-io/nats.c/blob/main/src/stan/pub.c method/function: natsStatus _stanPublish() line #: 267 code :

pa->guid = NATS_MALLOC(GUID_LEN); if (pa->guid == NULL) s = nats_setDefaultError(NATS_NO_MEMORY);

Question, shouldn't the last line be:

if (pa->guid == NULL) return nats_setDefaultError(NATS_NO_MEMORY);

If an allocation of GUID_LEN bytes fails then plenty of other lines of code in the remaining of the function will potentially fail because they possibly allocate more memory.

kozlovic commented 1 year ago

Question, shouldn't the last line be: if (pa->guid == NULL) return nats_setDefaultError(NATS_NO_MEMORY);

No, because we check for status after that line. It allows for unlocking and possible cleanup at the end of the function. Your suggestion is the prime example of why we are coding this way: you would have left a mutex locked by simply returning the error without unlocking the mutex locked line 260.