4 ms·

That kernel patch sets off some alarms. Dozens of goto statements and many of them are not the common C exit/cleanup idiom; backwards jumps are frequent and fo

That kernel patch sets off some alarms. Dozens of goto statements and many of them are not the common C exit/cleanup idiom; backwards jumps are frequent and following some of these functions is difficult. tls_sw_sendmsg and tls_sw_sendpage have 8 and 5 goto labels respectively. Ew. Might all be brilliant and flawless, but it's not obvious.

Thaxll 9y ago
Linux kernel has a lot of goto in its code.
- yahna 9y ago
  Other than jump to exit stuff?
  - cesarb 9y ago
    Yeah, the "goto retry" (jump back to the beginning of the function or the loop) is also common.
    - yahna 9y ago
      Still pretty structured usage.
cesarb 9y ago
As someone who has contributed a bit to the Linux kernel, I agree with you that the use of goto in these two functions is... unusual. They do seem to fall into a few patterns, however. On a first look at the first function (tls_sw_sendmsg), we appear to have: - send_end is the normal cleanup target at the end of the function, that's common and found almost everywhere; - alloc_encrypted, alloc_plaintext, push_record seem to be "retry" labels, where the code goes back and tries again; - fallback_to_reg_send is the "then" case of an if, could be removed by inverting the sense of the test; - wait_for_sndbuf, wait_for_memory are in the style of a cleanup target, but they end by jumping to one of the "retry" labels above; - trim_sgl is... well, it seems an attempt to avoid code duplication, or something like that, the "goto trim_sgl;" could be replaced by "trim_both_sgl(sk, orig_size); goto send_end;" Yeah, this code looks more like assembly than C. That appears to be the kind of hand-optimized code usually reserved for hot functions, where the "normal" case encounters as few branches as possible; the fallback_to_reg_send case points to that (the normal case would be the one without the goto). So the way to understand the control flow of these functions would be: ignore the gotos on first reading, since that would be the normal case, and after that look at each goto and understand what it does.

Thaxll 9y ago

Linux kernel has a lot of goto in its code.

yahna 9y ago
Other than jump to exit stuff?
- cesarb 9y ago
  Yeah, the "goto retry" (jump back to the beginning of the function or the loop) is also common.
  - yahna 9y ago
    Still pretty structured usage.

cesarb 9y ago

As someone who has contributed a bit to the Linux kernel, I agree with you that the use of goto in these two functions is... unusual. They do seem to fall into a few patterns, however. On a first look at the first function (tls_sw_sendmsg), we appear to have: - send_end is the normal cleanup target at the end of the function, that's common and found almost everywhere; - alloc_encrypted, alloc_plaintext, push_record seem to be "retry" labels, where the code goes back and tries again; - fallback_to_reg_send is the "then" case of an if, could be removed by inverting the sense of the test; - wait_for_sndbuf, wait_for_memory are in the style of a cleanup target, but they end by jumping to one of the "retry" labels above; - trim_sgl is... well, it seems an attempt to avoid code duplication, or something like that, the "goto trim_sgl;" could be replaced by "trim_both_sgl(sk, orig_size); goto send_end;" Yeah, this code looks more like assembly than C. That appears to be the kind of hand-optimized code usually reserved for hot functions, where the "normal" case encounters as few branches as possible; the fallback_to_reg_send case points to that (the normal case would be the one without the goto). So the way to understand the control flow of these functions would be: ignore the gotos on first reading, since that would be the normal case, and after that look at each goto and understand what it does.