On Wed, Nov 30, 2022 at 04:04:02PM +0100, 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.
Indicating that it indeed is meant to be an assignment is usually done
by wrapping it in parentheses.
> 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
^^^
The colon should stick to the left. Same for all other colons in both
patches.
> ---
> 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
