Hacker News Reader

>using both 0 and -1 to indicate error returns from functions

I couldn't find any function that returns 0 on failure. Or do you mean a null pointer?

>The stack should not be used to allocate buffers intended to hold data written by other routines for reasons that have become painfully clear over the last couple of years.

You want to see a zero tolerance policy for them? I think this is a bit overprotective. Their maximum size is usually known and all called functions take a size argument (snprintf(), strftime(), strlcpy(), etc.). There's a chance one could make a mistake here, but C is the wrong language to dynamically allocate everything. The probability to forget to free it, including in every error path, is much higher in my eyes. But then again, a memory leak is probably preferable than a buffer overflow (of course, both are bad). There are also already many heap allocated buffers.

>Looking a bit longer at the server_log code I think there may be a path to get a double free of 'ptr' where it is used first in the block with the while loop, then not reset to NULL and re-used in the second block and freed if it is not NULL (which it still is from the previous block...)

I don't think so, but that code really is dodgy. Also, `ptr != NULL` is unnecessary (just mentioning since OpenBSD/LibreSSL developers like to mention that too).

View on HN

int foo(void) { int ret = 0; /* Variable we return to indicate success or failure. Defaults to 0, which is success. */ FILE *inf; FILE *outf; if ((inf = fopen("foo", "rb")) == NULL) { ret = -1; goto inf_fail; } if ((outf = fopen("bar", "wb")) == NULL) { ret = -2; goto outf_fail; } /* Do the actual work, now that you know you have all the resources you need. */ fclose(outf); outf_fail: fclose(inf); /* If we jump here, we know we successfully opened inf, but not outf. */ inf_fail: /* If we jump here, we didn't actually open squat, so all we can do is return the status variable we set above. */ return ret; }

int foo(void) { int ret = -1; /* Variable we return to indicate success or failure. Defaults to -1, which is failure. */ FILE *inf; FILE *outf; if (!(inf = fopen("foo", "rb"))) { ret = -1; return ret; } do { if (!(outf = fopen("bar", "wb"))) { ret = -2; break; } do { /* Do the actual work, now that you know you have all the resources you need. Use break if something fails. */ ret = 0; } while(0); fclose(outf); } while(0); fclose(inf); return ret; }

int foo(void) { int ret = 0; /* Variable we return to indicate success or failure. Defaults to 0, which is success. */ FILE *inf = NULL; FILE *outf = NULL; if ((inf = fopen("foo", "rb")) == NULL) { ret = -1; goto foo_fail; } if ((outf = fopen("bar", "wb")) == NULL) { ret = -2; goto foo_fail; } /* Do the actual work, now that you know you have all the resources you need. */ foo_fail: if (outf) { fclose(outf); outf = NULL; } if (inf) { fclose(inf); inf = NULL; } return ret; }