Looks good to me
M.
On 12.02.2013 17:42, Denys Vlasenko wrote:
On 02/12/2013 03:14 PM, Michal Toman wrote:
+ safe_waitpid(xz_child, &status, 0);
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
+ {
+ error_msg_and_die(_("Can't create temporary file in /tmp"));
+ }
^^^ I can see some inconsistency here - on many places you put
the curly braces around a single function and on many you don't.
I usually go about it like this:
in the beginning:
if (expr)
something;
later needed to add stuff:
if (expr)
{
something1;
something2;
}
later had to remove stuff:
if (expr)
{
something2;
}
^^^^^^^^^^^^ I don't bother removing {},
what if next change will need to add them again?
In this case I copy-pasted the ifs from another C file,
they already had {}s and I didn't remove them,
as per above policy of "don't remove {}s is they exist".
Superfluous {}s would be less painful if we'd had {
on the "if" line... <wink> <wink>
+ err:
+ alert_server_error(NULL);
+ /* Show bad header to the user */
+ char *sanitized = sanitize_utf8(message, (SANITIZE_ALL & ~SANITIZE_TAB));
Do we really need to sanitize? IIRC HTTP headers are never UTF-8.
Either they are valid (ASCII) or totally broken and should probably not be
displayed.
I am doing this not so much because of UTF-8, but because I don't want
to end up with a message like this:
Malformed HTTP response header: 'HTTP/z garbage
'"
i.e. with newline inside ''! sanitize_utf8() would show it in hex.
-void alert_server_error();
-void alert_connection_error();
+void alert_server_error(const char *peer_name);
+void alert_connection_error(const char *peer_name);
You use 'peer_name' in the header and 'name' in the implementation.
Yes :)
