On Fri, Jul 23, 2004 at 01:10:42PM -0400, Garrett Kajmowicz wrote:
I have gone through and modified the https.c file to use (what I believe are) your preferred coding standards, with one exception.
Done. (i.e. commited).
I *cannot* stand unbraced if statements (or any other statements), and thus I braced them in two locations. I also don't care for use of the tertiary operator, but you wrote the function it is used in, so I don't plan on touching it.
No problem. Always-braced if statements probably is a better practice!
I would also like your opinion on how to "clean up" the function save_certificate_home() It has three exit points, and dropping it to fewer would be somewhat of a challenge to do cleanly.
OK, I'll look at it.
There are a few loctions where I believe you broke a statement which would ift on one line, specifically snprintf statements. I might have dont this (though I can't think of why). If this is the case, could you let me know your reasoning behind this? I'd like to learn from this if possible...
Those statements were originally in your patch! :-) <q> [...] + /*Attempt to create .dillo/certs blindly - check later*/ + snprintf(buf,4096,"%s/.dillo/", + g_get_home_dir() + ); + mkdir(buf, 01777); + snprintf(buf,4096,"%s/.dillo/certs/", + g_get_home_dir() + ); + mkdir(buf, 01777); + + do{ + snprintf(buf,4096,"%s/.dillo/certs/%x.%u\0", + g_get_home_dir(), + X509_subject_name_hash(cert), i + ); [...] </q> You can make them one line. --- BTW, g_free() tests for NULL so this statements: if (http_query != NULL){ g_free(http_query); http_query = NULL; } can simply become: g_free(http_query); because the variable is local, there's no need to NULL it. Except for 'root_url' which can also be made local. Finally, please send the patches to me directly (not to the list), but comments belong to the list. This saves some band width to hundreds of subscribers. Thanks for taking my advice. It'so much easier to review the patches now. Cheers Jorge.-