[PATCH v2] cmp: Only return success when both files have the same contents

[PATCH v2] cmp: Only return success when both files have the same contents

[Glenn Washburn]

From: Li Gen <ligenlive@gmail.com>

This allows the cmp command to be used in GRUB scripts to conditionally
run commands based on whether two files are the same.

Update documentation accordingly.

Signed-off-by: Li Gen <ligenlive@gmail.com>
Signed-off-by: Glenn Washburn <development@efficientek.com>
---
Range-diff against v1:
1:  5832522ca2 ! 1:  3e6c866d87 cmp: Only return success when both files have the same contents
    @@ Commit message
         This allows the cmp command to be used in GRUB scripts to conditionally
         run commands based on whether two files are the same.
     
    +    Update documentation accordingly.
    +
         Signed-off-by: Li Gen <ligenlive@gmail.com>
         Signed-off-by: Glenn Washburn <development@efficientek.com>
     
    + ## docs/grub.texi ##
    +@@ docs/grub.texi: bytes like this:
    + Differ at the offset 777: 0xbe [foo], 0xef [bar]
    + @end example
    + 
    +-If they are completely identical, nothing will be printed.
    ++If they are completely identical, nothing will be printed, and @code{$?}
    ++will be set to 0. Otherwise, if the files are not identical, @code{$?}
    ++will be set to a nonzero value.
    + @end deffn
    + 
    + 
    +
      ## grub-core/commands/cmp.c ##
     @@ grub-core/commands/cmp.c: grub_cmd_cmp (grub_command_t cmd __attribute__ ((unused)),
        grub_file_t file2 = 0;

 docs/grub.texi           | 4 +++-
 grub-core/commands/cmp.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index 4a6fb1a727..b12f47d351 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4486,7 +4486,9 @@ bytes like this:
 Differ at the offset 777: 0xbe [foo], 0xef [bar]
 @end example
 
-If they are completely identical, nothing will be printed.
+If they are completely identical, nothing will be printed, and @code{$?}
+will be set to 0. Otherwise, if the files are not identical, @code{$?}
+will be set to a nonzero value.
 @end deffn
 
 
diff --git a/grub-core/commands/cmp.c b/grub-core/commands/cmp.c
index e9c3b25d34..e1665cf27b 100644
--- a/grub-core/commands/cmp.c
+++ b/grub-core/commands/cmp.c
@@ -38,6 +38,7 @@ grub_cmd_cmp (grub_command_t cmd __attribute__ ((unused)),
   grub_file_t file2 = 0;
   char *buf1 = 0;
   char *buf2 = 0;
+  grub_err_t err = GRUB_ERR_TEST_FAILURE;
 
   if (argc != 2)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
@@ -91,6 +92,7 @@ grub_cmd_cmp (grub_command_t cmd __attribute__ ((unused)),
 
       /* TRANSLATORS: it's always exactly 2 files.  */
       grub_printf_ (N_("The files are identical.\n"));
+      err = GRUB_ERR_NONE;
     }
 
 cleanup:
@@ -102,7 +104,7 @@ cleanup:
   if (file2)
     grub_file_close (file2);
 
-  return grub_errno;
+  return err;
 }
 
 static grub_command_t cmd;
-- 
2.34.1

Re: [PATCH v2] cmp: Only return success when both files have the same contents

[Daniel Kiper]

On Thu, Oct 20, 2022 at 11:46:26PM -0500, Glenn Washburn wrote:
> From: Li Gen <ligenlive@gmail.com>
>
> This allows the cmp command to be used in GRUB scripts to conditionally
> run commands based on whether two files are the same.
>
> Update documentation accordingly.
>
> Signed-off-by: Li Gen <ligenlive@gmail.com>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
> Range-diff against v1:
> 1:  5832522ca2 ! 1:  3e6c866d87 cmp: Only return success when both files have the same contents
>     @@ Commit message
>          This allows the cmp command to be used in GRUB scripts to conditionally
>          run commands based on whether two files are the same.
>
>     +    Update documentation accordingly.
>     +
>          Signed-off-by: Li Gen <ligenlive@gmail.com>
>          Signed-off-by: Glenn Washburn <development@efficientek.com>
>
>     + ## docs/grub.texi ##
>     +@@ docs/grub.texi: bytes like this:
>     + Differ at the offset 777: 0xbe [foo], 0xef [bar]
>     + @end example
>     +
>     +-If they are completely identical, nothing will be printed.
>     ++If they are completely identical, nothing will be printed, and @code{$?}
>     ++will be set to 0. Otherwise, if the files are not identical, @code{$?}
>     ++will be set to a nonzero value.
>     + @end deffn
>     +
>     +
>     +
>       ## grub-core/commands/cmp.c ##
>      @@ grub-core/commands/cmp.c: grub_cmd_cmp (grub_command_t cmd __attribute__ ((unused)),
>         grub_file_t file2 = 0;
>
>  docs/grub.texi           | 4 +++-
>  grub-core/commands/cmp.c | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 4a6fb1a727..b12f47d351 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -4486,7 +4486,9 @@ bytes like this:
>  Differ at the offset 777: 0xbe [foo], 0xef [bar]
>  @end example
>
> -If they are completely identical, nothing will be printed.
> +If they are completely identical, nothing will be printed, and @code{$?}
> +will be set to 0. Otherwise, if the files are not identical, @code{$?}
> +will be set to a nonzero value.

I think the updated doc is not in line with the code.

>  @end deffn
>
>
> diff --git a/grub-core/commands/cmp.c b/grub-core/commands/cmp.c
> index e9c3b25d34..e1665cf27b 100644
> --- a/grub-core/commands/cmp.c
> +++ b/grub-core/commands/cmp.c
> @@ -38,6 +38,7 @@ grub_cmd_cmp (grub_command_t cmd __attribute__ ((unused)),
>    grub_file_t file2 = 0;
>    char *buf1 = 0;
>    char *buf2 = 0;
> +  grub_err_t err = GRUB_ERR_TEST_FAILURE;
>
>    if (argc != 2)
>      return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
> @@ -91,6 +92,7 @@ grub_cmd_cmp (grub_command_t cmd __attribute__ ((unused)),
>
>        /* TRANSLATORS: it's always exactly 2 files.  */
>        grub_printf_ (N_("The files are identical.\n"));

Did not we agree the cmp command should be quite by default?

> +      err = GRUB_ERR_NONE;
>      }
>
>  cleanup:
> @@ -102,7 +104,7 @@ cleanup:
>    if (file2)
>      grub_file_close (file2);
>
> -  return grub_errno;
> +  return err;
>  }
>
>  static grub_command_t cmd;

Daniel