[parted-devel] Parted on GNU Hurd based systems

[Jim Meyering]

"Debarshi 'Rishi' Ray" <debarshi.ray at gmail.com> wrote:
> How is this one?
>
> a. Used #ifndef..#endif on HAVE_SIGACTION and SA_SIGINFO to
> selectively define those structure and functions which will go missing
> when 'sigaction' and/or 'SA_SIGINFO' are absent. I have nested the two
> cases to make them more granular.
>
> b. Separately defined all the constants (as suggested by Jim) that are
> compared with info->si_code to print specific messages. Should work on
> FreeBSD kernels where some of these are absent.
>
> c. I have not used
> #define sigaction (a, b, c)
> Instead used a blank function definition, and a couple of blank
> structures to keep things readable.

Good.  A function is better, when the stronger parameter
type checking doesn't cause trouble.  I have to lose the macro reflex.
But make the function be "static inline".

> diff --git a/parted/ui.c b/parted/ui.c
> index 58a71a3..484d7c5 100644
> --- a/parted/ui.c
> +++ b/parted/ui.c
> @@ -63,6 +63,97 @@ extern int tgetnum (char* key);
>
>  #endif /* HAVE_LIBREADLINE */
>
> +#ifndef SA_SIGINFO
> +
> +#ifndef HAVE_SIGACTION

I find it more readable when cpp directives are indented
according to their nesting level.  E.g.

  #ifndef SA_SIGINFO
  # ifndef HAVE_SIGACTION

and

  #ifndef SEGV_MAPERR
  # define SEGV_MAPERR INTMAX - 1
  #endif

Whoops.  The above (and all the others) should parenthesize the sum:

  #ifndef SEGV_MAPERR
  # define SEGV_MAPERR (INTMAX - 1)
  #endif

>                  case SEGV_MAPERR:
> -                        fputs(_("\nError: SEGV_MAPERR (Address not mapped "
> -                                "to object)\n"), stdout);
> +                        fputs(_("\nError: SEGV_MAPERR "
> +                                "(Address not mapped to object)\n"), stdout);
>                          PED_ASSERT(0, break); /* Force a backtrace */

syntax-only changes like the above do not belong in a portability-related delta.
Sorry I didn't insist before, but they really should be separate.
They distract (and detract) from the semantic content of your patch.

A simple rule: a syntax-only change should always be in delta separate
from any semantic change.