Re: [Bulk] Re: [Bulk] Re: [PATCH] grub2: commands/cmp.c: grub_cmd_cmp()
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I haven't changed the BUFFER_SIZE (512). Please do if you think it is appropriate. 2005-06-29 Vincent Pelletier <[EMAIL PROTECTED]> * commands/cmp.c (BUFFER_SIZE): New macro. (grub_cmd_cmp): Close the right file at the right time. Compare only data just read. Don't report files of different size as identical. Dynamically allocate buffers. Move variable declarations at the beginning of function. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFCxmozFEQoKRQyjtURAhgIAJ9lR04VeEhMHnnTe1KJVorjRMzCbwCfYXDT 7qkbXSTMF7bGi8gaxS35/RI= =N0Ed -END PGP SIGNATURE- ___ Appel audio GRATUIT partout dans le monde avec le nouveau Yahoo! Messenger Téléchargez cette version sur http://fr.messenger.yahoo.com ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Bulk] Re: [PATCH] grub2: commands/cmp.c: grub_cmd_cmp()
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Marco Gerards wrote: > I think BUFFER_SIZE would be a better name. Another size can be > used. Perhaps it is better to use a bigger buffer size? As we read from a block device, I think the best size would be a multiple of the block size... But yes, it could be named buffer size. And it could even be dynamicaly allocated, to prevent using too much space on stack. > Huh? Some remains from a change-and-undo... I'll send a new patch with a correct changelog :). Vincent Pelletier -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFCw8j8FEQoKRQyjtURApAVAKCUiOSXjhRmv5Sy/Hkct/9rqZjapwCeNZws iYms73xiAlh3TMEgfIoctA4= =+ko/ -END PGP SIGNATURE- ___ Appel audio GRATUIT partout dans le monde avec le nouveau Yahoo! Messenger Téléchargez cette version sur http://fr.messenger.yahoo.com ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] grub2: commands/cmp.c: grub_cmd_cmp()
Vincent Pelletier <[EMAIL PROTECTED]> writes:
Hi Vincent,
Thanks for your patch. It looks ok to me. A few small comments...
> 2005-06-29 Vincent Pelletier <[EMAIL PROTECTED]>
>
> * commands/cmp.c
> (grub_cmd_cmp): Close the right file at the right time. Compare
> only data just read. Don't report files of different size as
> identical. (BLOCK_SIZE): New macro.
The (BLOCK_SIZE) stuff should be put on a new line.
I think BUFFER_SIZE would be a better name. Another size can be
used. Perhaps it is better to use a bigger buffer size?
>
>if (grub_file_size (file1) != grub_file_size (file2))
> -grub_printf ("Differ in size: %d [%s], %d [%s]\n",
> - grub_file_size (file1), args[0],
> - grub_file_size (file2), args[1]);
> +{
> + grub_printf ("Differ in size: %d [%s], %d [%s]\n",
> +grub_file_size (file1), args[0],
> +grub_file_size (file2), args[1]);
> +}
Huh?
Thanks,
Marco
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] grub2: commands/cmp.c: grub_cmd_cmp()
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Vincent Pelletier wrote:
> nb: I haven't tested these changes beyond "make", but I think they are
> trivial enough to be trusted...
My baaad...
There were 2 bugs left :
- -If the file size isn't a multiple of 512 bytes, we would read
non-initialised or non-updated memory (so it only affects results on
files whom size is < 512)
- -"The files are identical." is displayed when file size differ (was
corrected in Wanderley's version)
Reminds me Hagakure :
"Matters of small concern should be considered seriously".
I'm not sure if it is right to add a macro definition for block size,
and wether the name is good.
2005-06-29 Vincent Pelletier <[EMAIL PROTECTED]>
* commands/cmp.c
(grub_cmd_cmp): Close the right file at the right time. Compare
only data just read. Don't report files of different size as
identical. (BLOCK_SIZE): New macro.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.1 (GNU/Linux)
iD8DBQFCwsVfFEQoKRQyjtURApNOAKC4bO41ca3vX/m64aIy4hIvjIwW1QCgmbyg
AIn3yvP072FMa0LlDFo1CYM=
=o8fq
-END PGP SIGNATURE-
Index: commands/cmp.c
===
RCS file: /cvsroot/grub/grub2/commands/cmp.c,v
retrieving revision 1.2
diff -u -p -r1.2 cmp.c
--- commands/cmp.c 4 Apr 2004 13:46:00 - 1.2
+++ commands/cmp.c 29 Jun 2005 15:47:36 -
@@ -24,6 +24,8 @@
#include
#include
+#define BLOCK_SIZE 512
+
static grub_err_t
grub_cmd_cmp (struct grub_arg_list *state __attribute__ ((unused)),
int argc, char **args)
@@ -44,54 +46,53 @@ grub_cmd_cmp (struct grub_arg_list *stat
file2 = grub_file_open (args[1]);
if (! file2)
{
- grub_file_close (file2);
+ grub_file_close (file1);
return grub_errno;
}
if (grub_file_size (file1) != grub_file_size (file2))
-grub_printf ("Differ in size: %d [%s], %d [%s]\n",
-grub_file_size (file1), args[0],
-grub_file_size (file2), args[1]);
+{
+ grub_printf ("Differ in size: %d [%s], %d [%s]\n",
+ grub_file_size (file1), args[0],
+ grub_file_size (file2), args[1]);
+}
else
{
- char buf1[512];
- char buf2[512];
+ char buf1[BLOCK_SIZE];
+ char buf2[BLOCK_SIZE];
grub_ssize_t rd1, rd2;
grub_uint32_t pos = 0;
do
{
int i;
- rd1 = grub_file_read (file1, buf1, 512);
- rd2 = grub_file_read (file2, buf2, 512);
+ rd1 = grub_file_read (file1, buf1, BLOCK_SIZE);
+ rd2 = grub_file_read (file2, buf2, BLOCK_SIZE);
if (rd1 != rd2)
- return 0;
+ goto cleanup;
- for (i = 0; i < 512; i++)
+ for (i = 0; i < rd1; i++)
{
if (buf1[i] != buf2[i])
{
grub_printf ("Differ at the offset %d: 0x%x [%s], 0x%x
[%s]\n",
i + pos, buf1[i], args[0],
buf2[i], args[1]);
-
- grub_file_close (file1);
- grub_file_close (file2);
- return 0;
+ goto cleanup;
}
}
- pos += 512;
+ pos += BLOCK_SIZE;
} while (rd2);
+ grub_printf ("The files are identical.\n");
}
+cleanup:
grub_file_close (file1);
grub_file_close (file2);
- grub_printf ("The files are identical.\n");
-
return 0;
}
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
