Re: [RFC][PATCH] pkl: make PK_UNREACHABLE more verbose

[Arsen Arsenović]

Hi Mohammad,

This is a nice change.

Mohammad-Reza Nabipoor <mnabipoor@gnu.org> writes:

> 2023-02-13  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>
>       * common/pk-utils.h (pk_unreachable): New function declaration.
>       (PK_UNREACHABLE): Changed the macro to use `pk_unreachable'.
>       * common/pk-utils.c (pk_unreachable): New function definition.
> ---
>
> Hello Jose and Bruno and Poke folks!
>
> In this patch I improved the `PK_UNREACHABLE' a little bit, but ...
>
> Do you think it's a good idea to use `setjmp'/`longjmp' to handle
> these kind of internal errors?
>
> Because libpoke is a library, and it's better for libraries to not
> abort.
>
>
> Regards,
> Mohammad-Reza
>
>
>  ChangeLog         |  6 ++++++
>  common/pk-utils.c |  7 +++++++
>  common/pk-utils.h | 16 +++++++++-------
>  3 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index afdd66ef..6513c682 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,9 @@
> +2023-02-13  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
> +
> +     * common/pk-utils.h (pk_unreachable): New function declaration.
> +     (PK_UNREACHABLE): Changed the macro to use `pk_unreachable'.
> +     * common/pk-utils.c (pk_unreachable): New function definition.
> +
>  2023-02-12  Jose E. Marchesi  <jemarch@gnu.org>
>  
>       * common/pk-utils.h: Include assert.h.
> diff --git a/common/pk-utils.c b/common/pk-utils.c
> index 612f8a89..342c16cf 100644
> --- a/common/pk-utils.c
> +++ b/common/pk-utils.c
> @@ -222,3 +222,10 @@ pk_str_trim (char **str)
>    while (isspace (*--end));
>    *(end + 1) = '\0';
>  }
> +
> +void pk_unreachable (const char *filename, int line)
> +{
> +  fprintf (stderr, "unreachable reached (%s:%d)\n", filename, line);

Can we also introduce the function name here?  Line numbers, in general,
aren't too useful, and, assuming that a function has few unreachable
points, the name of the function that reached unreachable is probably a
better identifier.  Best to have both, though, naturally ;)

> +  fflush (NULL);
> +  abort ();
> +}
> diff --git a/common/pk-utils.h b/common/pk-utils.h
> index a4dba8e1..f6a46086 100644
> --- a/common/pk-utils.h
> +++ b/common/pk-utils.h
> @@ -66,13 +66,15 @@ char *pk_str_replace (const char *in, const char *search, 
> const char *replace);
>  /* Left and rigth trim the given string from whitespaces.  */
>  void pk_str_trim (char **str);
>  
> +/* This function is called when the program reaches a supposedly
> +   unreachable point; print an error message and abort the execution.
> +
> +   FILENAME and LINE are the location information of invocation
> +   of this function.  */
> +void pk_unreachable (const char *filename, int line)
> +    __attribute__ ((noreturn));
> +
>  /* Diagnoses reaching unreachable code, and aborts.  */
> -#define PK_UNREACHABLE() \
> -  do                                                    \
> -    {                                                   \
> -      assert (0 && "Reached unreachable code.");        \
> -      abort ();                                         \
> -    }                                                   \
> -  while (0)
> +#define PK_UNREACHABLE() pk_unreachable (__FILE__, __LINE__)
>  
>  #endif /* ! PK_UTILS_H */

Have a lovely day.
-- 
Arsen Arsenović

signature.asc
Description: PGP signature