4 ms·

>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 stac

• jacquesm 12y ago
  > I couldn't find any function that returns 0 on failure. Or do you mean a null pointer? server_socket_getport An unknown address family will return a '0'. > You want to see a zero tolerance policy for them? No, but if you use that mechanism then it is preferable to have all the functions hitting that buffer to be visible from the scope of the declaration of the buffer. Passing it on to other libraries can cause problems when/if those libraries' maintainers mess up. So if you're defensive about this then you can't be hurt that way. > I don't think so, Agreed, looking still longer it looks like it will always end up with a NULL in it after the while, but that's very ugly to put it mildly. > Also, `ptr != NULL` is unnecessary (just mentioning since OpenBSD/LibreSSL developers like to mention that too). Yep. And there are plenty of other nitpicks like that but I don't even care that much about any of those, I mostly care about the way that the code is laid out making it an excellent place to hide some really nasty bugs.
  • cremno 12y ago
    >server_socket_getport I found this interesting article: http://daniel.haxx.se/blog/2014/10/25/pretending-port-zero-is-a-normal-one/ http://daniel.haxx.se/blog/2014/10/25/pretending-port-zero-i... So since 0 can be okay and (in_port_t)-1 (=65535) is okay, it has to be changed to return int and take a in_port_t* to be acceptable for you. The return value may not even matter if only AF_INET/AF_INET6 as family is possible - a comment could be of help here.
  • erkl 12y ago
    > An unknown address family will return a '0'. In this particular case, returning 0 doesn't necessarily indicate failure. Binding a socket to port 0 means you're asking the operating system to pick an available port for you, which one might argue is a reasonably safe default for unknown address families.
    • jacquesm 12y ago
      No, there is no 'default' at play here. If you don't know what address family is in use then you should simply abort rather than to let the end user of your product guess that the address family code is the culprit. Let it crash, as close as possible to the point of origin of a problem is a very good principle.
• cbd1984 12y ago
  > 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. There's a trick you can use to make remembering to free it more likely. Some people call it "RAII in C", I like to think of it as "nested allocations and errors": When you allocate something, or do something else which one, can fail, and two, must be undone (freed, closed, etc.), do the allocation and deallocation in properly nested pairs, and use gotos to jump to the code which undoes the last successful allocation if some allocation goes wrong. Basically, you arrange your code in a stack, where the most recent successful allocation is undone first, so you can neatly jump down to the code which undoes all successful allocations and doesn't try to undo the unsuccessful ones. For example: 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; } C++ does basically this automatically, which is called RAII, but in C you have to do it by hand.
  • edwintorok 12y ago
    Trying to figure out control flow with more than one goto label in a function is going to get complicated. I prefer to use `do { } while 0`: 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; } If you really must use gotos then just one suffices if you initialize all your variables to NULL: 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; } P.S. the return code of fclose() should also be checked, as I/O errors might only be reported on close