I highly appreciate both proposals. It makes the code clear and less error-prone.
Best regards, Helge On 30.11.2022 16:04, fenug...@mail.com wrote:
I think the HACKING file in the different sigrok projects could be updated and unified. Here is one step in that direction : (also submitted as https://github.com/sigrokproject/libsigrok/pull/199 ) In there, I add two non-binding recommendations : 1- avoid bundling an assignment in a conditional, `if ((var = func()) > other_var) ....` This makes reviews annoying as one needs to look twice to see if it was meant to be '==', and if not, unwrap the logic from the assignment. The codebase isn't uniform in this matter but I think any new code should follow the simpler pattern. 2- use read_be16() & similar helpers instead of other options (RB16, htons, hand-rolled...) Comments welcome (I will check here occasionally but prefer github). If this is acceptable I will submit similar patches for other subprojects. ********************** patches below ( 2 commits) From 5e54d3f76351bd5fd1054bf4d90a4882b5334121 Mon Sep 17 00:00:00 2001 From: fenugrec <fenug...@users.sourceforge.net> Date: Wed, 30 Nov 2022 09:27:20 -0500 Subject: [PATCH 1/2] HACKING : separate assignments from control flow --- HACKING | 3 +++ 1 file changed, 3 insertions(+) diff --git a/HACKING b/HACKING index 9d71c55c..e7158db0 100644 --- a/HACKING +++ b/HACKING @@ -79,6 +79,9 @@ Random notes - Generally avoid assigning values to variables at declaration time, especially so for complex and/or run-time dependent values. + - Separate assignments from control flow. Example : avoid the pattern + if (var = func()) {...} as it complicates review and maintenance. + - Consistently use g_*malloc() / g_*malloc0(). Do not use standard malloc()/calloc() if it can be avoided (sometimes other libs such as libftdi can return malloc()'d memory, for example). -- 2.38.1 ************************ From 4cf7cd961ef48c2dc8f5898ca81be29b8b9fdb7f Mon Sep 17 00:00:00 2001 From: fenugrec <fenug...@users.sourceforge.net> Date: Wed, 30 Nov 2022 09:33:34 -0500 Subject: [PATCH 2/2] HACKING : note preferred endianness helpers --- HACKING | 3 +++ 1 file changed, 3 insertions(+) diff --git a/HACKING b/HACKING index e7158db0..025b9ac9 100644 --- a/HACKING +++ b/HACKING @@ -98,6 +98,9 @@ Random notes Do use g_try_malloc() or g_try_malloc0() for large (>= 1MB) allocations and check the return value. + - Endianness conversions : prefer the functions provided in + libsigrok-internal.h, such as read_be16() etc. + - You should never print any messages (neither to stdout nor stderr nor elsewhere) "manually" via e.g. printf() or g_log() or similar functions. Only sr_err()/sr_warn()/sr_info()/sr_dbg()/sr_spew() should be used. -- 2.38.1 _______________________________________________ sigrok-devel mailing list sigrok-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/sigrok-devel
_______________________________________________ sigrok-devel mailing list sigrok-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/sigrok-devel
