5 ms·

Out of curiosity, how do you come around assessing it as high-quality code? By what measure(s)?

loup-vaillant 9y ago
I have developed some kind of instinct that is non-trivial to teach, and impossible to convey in a small HN comment. But my primary proxy is size. The less code the better. But even that have to give way to other concerns, such as style constraints, and straight up performance. While I can't show the code I was talking about above (it was proprietary), I can show my crypto library, Monocypher¹. I think it is a good example of what I mean by high-quality code. (Yes, I am boasting. I believe this is justified. Also, quality requirements for crypto code are kinda off the chart.) [1]: https://github.com/LoupVaillant/Monocypher https://github.com/LoupVaillant/Monocypher
- joshuamorton 9y ago
  I think crypto code is a bad example. This[1] would not be considered "idiomatic" anywhere, except in crypto. And for example, if one was optimizing for code readability over performance, I expect that would look very different (although you could do much of the same with macros, but I'm not sure if that's any better). [1]: https://github.com/LoupVaillant/Monocypher/blob/master/src/monocypher.c#L1070 https://github.com/LoupVaillant/Monocypher/blob/master/src/m...
  - loup-vaillant 9y ago
    Each domain has its own quirks. I'd definitely use a different style (even a different language) for other domains. I thought of showing some of my OCaml code as well, but I didn't put nearly as much effort, so I'm not so sure about its quality.
- capitalsigma 9y ago
  #define FOR(i, start, end) for (size_t (i) = (start); (i) < (end); (i)++) yuck EDIT: Also, what the hell is this macro defined in the middle of a function and then only used twice: https://github.com/LoupVaillant/Monocypher/blob/master/src/monocypher.c#L1091 https://github.com/LoupVaillant/Monocypher/blob/master/src/m... Why do you need this function for "constant time comparison to 0": https://github.com/LoupVaillant/Monocypher/blob/master/src/monocypher.c#L68 https://github.com/LoupVaillant/Monocypher/blob/master/src/m... static int neq0(u64 diff) { // constant time comparison to zero // return diff != 0 ? -1 : 0 u64 half = (diff >> 32) | ((u32)diff); return (1 & ((half - 1) >> 32)) - 1; } I am incredibly skeptical that you've done better than the compiler with this bit twiddling stuff. You realize that on x86_64, it probably compiles down to 1-2 instructions to write `diff != 0`, right?
  - lmz 9y ago
    > You realize that on x86_64, it probably compiles down to 1-2 instructions to write `diff != 0`, right? But will the 1-2 instruction compare be constant time?
    - lsiebert 9y ago
      constant time comparison in crypto is potentially important because of timing attacks to gain information. Haven't looked at the code here to see if that's the case, nor am I a crypto expert, but see https://en.wikipedia.org/wiki/Timing_attack https://en.wikipedia.org/wiki/Timing_attack
      - lmz 9y ago
        I am aware of that. I'm not sure about the poster claiming it reduces to "1-2 instructions".
    - loup-vaillant 9y ago
      They likely won't. They may compile into a conditional branch, which because of cache and pipeline invalidation do not run in constant time.
  - bri3d 9y ago
    This isn't an effort to beat the compiler on speed or optimization - rather, the opposite. It's an effort to produce a method which executes in the exact same amount of time for any input - whether or not it is zero. This is key in implementing crypto while avoiding side-channel timing attacks and this isn't a particularly uncommon way to do so. Now, with that being said, this strikes me as a domain issue: the OP seems well-versed in crypto and foundation / backend code, which has relatively constrained input and output behaviors and is testable using smart reasoning and fuzzing. This is somewhat different from front-end or user-facing code, which involves handling epic quantities of mutable state and requires validation of a feature end-to-end. I suspect OP writes great, "bug-free" code in the backend / systems domain, but I don't think their testing practice or anecdotes are applicable across the industry.

I have developed some kind of instinct that is non-trivial to teach, and impossible to convey in a small HN comment. But my primary proxy is size. The less code the better. But even that have to give way to other concerns, such as style constraints, and straight up performance. While I can't show the code I was talking about above (it was proprietary), I can show my crypto library, Monocypher¹. I think it is a good example of what I mean by high-quality code. (Yes, I am boasting. I believe this is justified. Also, quality requirements for crypto code are kinda off the chart.) [1]: https://github.com/LoupVaillant/Monocypher https://github.com/LoupVaillant/Monocypher

joshuamorton 9y ago
I think crypto code is a bad example. This[1] would not be considered "idiomatic" anywhere, except in crypto. And for example, if one was optimizing for code readability over performance, I expect that would look very different (although you could do much of the same with macros, but I'm not sure if that's any better). [1]: https://github.com/LoupVaillant/Monocypher/blob/master/src/monocypher.c#L1070 https://github.com/LoupVaillant/Monocypher/blob/master/src/m...
- loup-vaillant 9y ago
  Each domain has its own quirks. I'd definitely use a different style (even a different language) for other domains. I thought of showing some of my OCaml code as well, but I didn't put nearly as much effort, so I'm not so sure about its quality.
capitalsigma 9y ago
#define FOR(i, start, end) for (size_t (i) = (start); (i) < (end); (i)++) yuck EDIT: Also, what the hell is this macro defined in the middle of a function and then only used twice: https://github.com/LoupVaillant/Monocypher/blob/master/src/monocypher.c#L1091 https://github.com/LoupVaillant/Monocypher/blob/master/src/m... Why do you need this function for "constant time comparison to 0": https://github.com/LoupVaillant/Monocypher/blob/master/src/monocypher.c#L68 https://github.com/LoupVaillant/Monocypher/blob/master/src/m... static int neq0(u64 diff) { // constant time comparison to zero // return diff != 0 ? -1 : 0 u64 half = (diff >> 32) | ((u32)diff); return (1 & ((half - 1) >> 32)) - 1; } I am incredibly skeptical that you've done better than the compiler with this bit twiddling stuff. You realize that on x86_64, it probably compiles down to 1-2 instructions to write `diff != 0`, right?
- lmz 9y ago
  > You realize that on x86_64, it probably compiles down to 1-2 instructions to write `diff != 0`, right? But will the 1-2 instruction compare be constant time?
  - lsiebert 9y ago
    constant time comparison in crypto is potentially important because of timing attacks to gain information. Haven't looked at the code here to see if that's the case, nor am I a crypto expert, but see https://en.wikipedia.org/wiki/Timing_attack https://en.wikipedia.org/wiki/Timing_attack
    - lmz 9y ago
      I am aware of that. I'm not sure about the poster claiming it reduces to "1-2 instructions".
  - loup-vaillant 9y ago
    They likely won't. They may compile into a conditional branch, which because of cache and pipeline invalidation do not run in constant time.
- bri3d 9y ago
  This isn't an effort to beat the compiler on speed or optimization - rather, the opposite. It's an effort to produce a method which executes in the exact same amount of time for any input - whether or not it is zero. This is key in implementing crypto while avoiding side-channel timing attacks and this isn't a particularly uncommon way to do so. Now, with that being said, this strikes me as a domain issue: the OP seems well-versed in crypto and foundation / backend code, which has relatively constrained input and output behaviors and is testable using smart reasoning and fuzzing. This is somewhat different from front-end or user-facing code, which involves handling epic quantities of mutable state and requires validation of a feature end-to-end. I suspect OP writes great, "bug-free" code in the backend / systems domain, but I don't think their testing practice or anecdotes are applicable across the industry.