On Fri, 05 Mar 2021 10:18:07 +0100
raistlin.df@xxxxxxxxx wrote:
> For Stefano: about the patch itself, I think the only thing I'm going
> to say is that some of the lines look a bit long.
>
> I do not see a CODING_STYLE file in the repo, but just looking around,
> I got the impression that we should stay within 76-80 characters all
> the times that it is possible.
Honestly, I'm not a fan of the 80 character limit. I don't want lines that
are 200 characters long either. My windows are normally between 90 and 100
characters wide, so keeping it under 100 is fine with me. Especially if it
keeps the code looking better.
For example:
+ current_trace_id = tracecmd_get_traceid(handles->handle);
+ if (tracecmd_get_guest_cpumap(peer_handle, current_trace_id,
+ NULL, NULL, NULL) != -1)
+ tracecmd_pair_peer(handles->handle, peer_handle);
Should be either:
current_trace_id = tracecmd_get_traceid(handles->handle);
if (tracecmd_get_guest_cpumap(peer_handle, current_trace_id, NULL, NULL, NULL) != -1)
tracecmd_pair_peer(handles->handle, peer_handle);
or:
current_trace_id = tracecmd_get_traceid(handles->handle);
if (tracecmd_get_guest_cpumap(peer_handle, current_trace_id,
NULL, NULL, NULL) != -1) {
tracecmd_pair_peer(handles->handle, peer_handle);
}
Two changes above:
If you break a if conditional up, that categorizes it as a "complex"
statement and brackets should be used. (This is also for Linux kernel
coding style).
The second is, if you break up a function call parameters, the parameters
starting on the next line should line up with the starting parenthesis.
Note, this is not always the case. If the broken line is still long, it is
OK to have the line start before the parenthesis to keep it symmetrical:
current_trace_id = tracecmd_get_traceid(handles->handle);
if (tracecmd_get_guest_cpumap(peer_handle, current_trace_id,
some_string, another_string, bigger_string_as_well) != -1) {
tracecmd_pair_peer(handles->handle, peer_handle);
}
But that too should be avoided if possible.
Thanks!
-- Steve
|