[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
grub2: commands/cmp.c: grub_cmd_cmp()
From: |
Rodrigo Steinmüller Wanderley |
Subject: |
grub2: commands/cmp.c: grub_cmd_cmp() |
Date: |
Wed, 29 Jun 2005 08:33:58 -0300 |
Good Morning!
I'm reading grub2 commands in order to understand the syntax to add new
builtins into it. When reading the cmp commands I found some points I
couldn't understand, think they are bugs, but maybe some of you guys can
prove me wrong...
I will comment the points I didn't understand here:
static grub_err_t
grub_cmd_cmp (struct grub_arg_list *state __attribute__ ((unused)),
int argc, char **args)
{
/* ... */
file1 = grub_file_open (args[0]);
if (! file1)
return grub_errno;
file2 = grub_file_open (args[1]);
if (! file2)
{
grub_file_close (file2);
*** Shouldn't we be closing file1 here? ***
return grub_errno;
}
/* ... */
do
{
int i;
rd1 = grub_file_read (file1, buf1, 512);
rd2 = grub_file_read (file2, buf2, 512);
if (rd1 != rd2)
*** We are returning without closing file1 and file2 ***
return 0;
for (i = 0; i < 512; 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);
*** I think here is ok, but wouldn't it be better to use a goto and have
*** a single exit point here?
return 0;
}
}
pos += 512;
} while (rd2);
}
grub_printf ("The files are identical.\n");
return 0;
}
What I'm thinking is something like (even though this function still
looks somewhat ugly to me):
static grub_err_t
grub_cmd_cmp (struct grub_arg_list *state __attribute__ ((unused)),
int argc, char **args)
{
grub_file_t file1 = 0;
grub_file_t file2 = 0;
grub_err_t err = 0;
if (argc != 2)
return grub_error (GRUB_ERR_BAD_ARGUMENT, "two arguments required");
grub_printf ("Compare `%s' and `%s':\n", args[0],
args[1]);
file1 = grub_file_open (args[0]);
if (! file1) {
err = grub_errno;
goto finish;
}
file2 = grub_file_open (args[1]);
if (! file2)
{
err = grub_errno;
goto finish;
}
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]);
else
{
char buf1[512];
char buf2[512];
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);
if (rd1 != rd2)
goto finish;
for (i = 0; i < 512; 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]);
goto finish;
}
}
pos += 512;
} while (rd2);
grub_printf ("The files are identical.\n");
}
finish:
if (file1)
grub_file_close (file1);
if (file2)
grub_file_close (file2);
return err;
}
Best Regards,
Rodrigo Steinmuller Wanderley
--
-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: GnuPG v1.4.1 (GNU/Linux)
mQGiBEKJ288RBAD43+VlxMx8V0dLbU+f7TsbhknjBYp2sRMP0a8IkHa8z4DgJTRd
XRMB0D05Hp5iE/1cA8t3e+g2J4kQhcj1JgUA6KSpYcj/cX6EKb6xhb/GAEQupaXz
7RYglwf4Sz9WJA3roSLtQuWcCOYR9lys+kifeTE2jnDLzDcuzwa2pEYJbwCg04jF
uyOmiBd09P1Bgq4VOQhYM78D/j0Iyj0QIstssnRPWcg4QL9l5c7Y8rLRH63qfGOi
fakmmY6C1JnW/wm4+2iUOc0/DbM+kKS5yXsiRFW7CDeqXLUEF1NIRvNaHkmfRmQf
shDI8NJCr0ULMbUde3b1U0LKgMRr7uVnVRFb2bPkEFh1mDEaxpy376+2Rpn8uHOu
GvqYBACJzY7EPP0fFQMMxeSyxHA7A/lxmC9/s1YtRgBHTCniYOQIZ+kwbFrU9XQv
ExvMeO2DvYAtDNyCgV/PaUm0yLxCAmxSVxQaAMRkOuMSKatyBggLpJVZKQ4WuayL
3xA+ws2+F2ozC/LHK9DodkGen35lP286QyPXOV2WciE4YciU3bQsUm9kcmlnbyBT
IFdhbmRlcmxleSA8cndhbmRlcmxleUBuYXRhbG5ldC5icj6IZAQTEQIAJAUCQonb
zwIbAwUJAeEzgAYLCQgHAwIDFQIDAxYCAQIeAQIXgAAKCRBuM/JKbknmQugDAKC6
ZfWsa8qone19+oppGBkrX028QACfUfLi9rSs/qxmE77b0P+xa2IrWN25Ag0EQonb
2xAIANBkeWLFcVSxSCsCQEH8HJ80VhQO18Sy80MpXebf9sj1gwUATZJ/OcxYYw46
ZrFwNk9raTRULprAcqR5ORKk3TNZ6ZnEl337PZZS5FnELwsHXTm+KVKF3bE2nnB5
/25SzPwkidsyk8Pe3HYM9/r4dwHNOXE3i0nYsweC/aUE8yg/3Ipweu9K1cj+XbSM
IpDydOmBpvVhIvv+VOIoevXxgm2hrD7LQ7jnfBaj/bV9GY/tJyl50nWgMM7csaAg
+4H1lG5/FvzNOgudmhzAdMk5lyTMLyRj6wiYkvckvBCXFaC04FgseylRj72NZilQ
xIstJWNomiATkC6uHYtOKExZ1xcAAwUIALTSG+l21w/W3L9iuEi8QK91n7LyHoO/
OJpYbj73sJWsui7qG63os8aR+KgbdbKNFGDwkyYfbfildYDd+TOkFWkbT64vq4Wv
t51Pl2dB0+0cnO/xqRnbxt4II7SBwg5t1u/MHahaULoTcTYslN+bW9FuB9I22ZiJ
pzFddDWjWApggNQIEapCd+XiuYnED6rV+n0GcmZxpb9Iz0mak7SPCZvN3QzPCI/6
k2YZlt92I/k4E2GU9NVM/1mXkTgqVgwOwlunPW6JYgcv/3n2Ly1eMNJQioWGRSnZ
wQyVx7FvBUqMGLrWHTw3+FQRDd6B6pQ2Y4uL0W4LskQrXm97hhW5NuKITwQYEQIA
DwUCQonb2wIbDAUJAeEzgAAKCRBuM/JKbknmQuXzAKDTa3d+h15/KHHupI6AMkNr
YKRP3ACggEq09XZBLGulCU2e6+/I0j4iN3U=
=JhUw
-----END PGP PUBLIC KEY BLOCK-----
- grub2: commands/cmp.c: grub_cmd_cmp(),
Rodrigo Steinmüller Wanderley <=