Re: [PATCH] caseless uuid detection, fixed wrong behaviour for strncasecmp, added strcasecmp
On Tue, 27 Jan 2009 23:02:52 -0500 Pavel Roskin pro...@gnu.org wrote: P.S. The reuse would be impractical. fs_uuid is a separate module, not the core. Reusing search_fs_uuid() would make the search module depend on fs_uuid. In my opinion, it's not worth the trouble to introduce that dependency. It's not like we would save a lot of code. Inlining or macro may be considered at that point. (I.e., reusing code via .h file.) If someone will try to change such function (e.g., to provide HisNewFs) then he need not to patch two places. -- Tomáš 'ebík' Ebenlendr http://get.to/ebik PF 2009.07504223744 ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] caseless uuid detection, fixed wrong behaviour for strncasecmp, added strcasecmp
On Mon, 2009-01-26 at 19:28 +0100, Daniel Mierswa wrote: On 26.01.2009 05:41, Pavel Roskin wrote: I'll appreciate if you write your Changelog entries according to the GNU coding standards. In particular, please don't abbreviate function names. Ok, second try. I should have told you that I had applied your patch with rewritten ChangeLog entries. A quick look at your second try shows that you didn't check the GNU coding standards. You can find it using Google or another search engine. The part dealing with ChangeLogs is here: http://www.gnu.org/prep/standards/standards.html#Change-Logs We have a very similar function search_fs_uuid() in disk/fs_uuid.c, and it still uses grub_strcmp(). Should it be using grub_strcasecmp() as well? Can we export that function to reuse it in commands/search.c? Yes it should use grub_strcasecmp, thanks for noticing. I don't know exactly if we can reuse that function at another place. I leave that to someone who knows where this code is executed. :-) The code is executed when the disk is referenced by its UUID. OK, I'll take care of it. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] caseless uuid detection, fixed wrong behaviour for strncasecmp, added strcasecmp
On Tue, 2009-01-27 at 20:49 -0500, Pavel Roskin wrote: We have a very similar function search_fs_uuid() in disk/fs_uuid.c, and it still uses grub_strcmp(). Should it be using grub_strcasecmp() as well? Can we export that function to reuse it in commands/search.c? P.S. The reuse would be impractical. fs_uuid is a separate module, not the core. Reusing search_fs_uuid() would make the search module depend on fs_uuid. In my opinion, it's not worth the trouble to introduce that dependency. It's not like we would save a lot of code. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] caseless uuid detection, fixed wrong behaviour for strncasecmp, added strcasecmp
On Fri, 2009-01-23 at 10:51 +0100, Daniel Mierswa wrote: Am 01/21/09 18:30, Pavel Roskin schrieb: The patch looks good to me. I would split changes to commands/search.c into a separate commit. Please provide ChangeLog entries for the patches. I will comply. Thanks for your quick response. I'll appreciate if you write your Changelog entries according to the GNU coding standards. In particular, please don't abbreviate function names. 2009-01-23 Daniel Mierswa impu...@impulze.org * commands/search.c: caseless UUID comparing We have a very similar function search_fs_uuid() in disk/fs_uuid.c, and it still uses grub_strcmp(). Should it be using grub_strcasecmp() as well? Can we export that function to reuse it in commands/search.c? -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] caseless uuid detection, fixed wrong behaviour for strncasecmp, added strcasecmp
On 26.01.2009 05:41, Pavel Roskin wrote:
I'll appreciate if you write your Changelog entries according to the GNU
coding standards. In particular, please don't abbreviate function
names.
Ok, second try.
We have a very similar function search_fs_uuid() in disk/fs_uuid.c, and
it still uses grub_strcmp(). Should it be using grub_strcasecmp() as
well? Can we export that function to reuse it in commands/search.c?
Yes it should use grub_strcasecmp, thanks for noticing. I don't know
exactly if we can reuse that function at another place. I leave that to
someone who knows where this code is executed. :-)
--
Mierswa, Daniel
If you still don't like it, that's ok: that's why I'm boss. I simply
know better than you do.
--- Linus Torvalds, comp.os.linux.advocacy, 1996/07/22
Index: commands/search.c
===
--- commands/search.c (revision 1954)
+++ commands/search.c (working copy)
@@ -115,7 +115,7 @@
(fs-uuid) (dev, uuid);
if (grub_errno == GRUB_ERR_NONE uuid)
{
- if (grub_strcmp (uuid, key) == 0)
+ if (grub_strcasecmp (uuid, key) == 0)
{
/* Found! */
count++;
Index: disk/fs_uuid.c
===
--- disk/fs_uuid.c (revision 1954)
+++ disk/fs_uuid.c (working copy)
@@ -52,7 +52,7 @@
{
(*count)++;
- if (grub_strcmp (uuid, key) == 0)
+ if (grub_strcasecmp (uuid, key) == 0)
{
ret = dev;
grub_free (uuid);
2009-01-23 Daniel Mierswa impu...@impulze.org
* kern/misc.c: add grub_strcasecmp for consistency reasons, use
grub_size_t
instead of int for str-functions, fix grub_strncasecmp
return values, use the same algorithm in both
str-functions
* include/grub/misc.h: add grub_strcasecmp, use grub_size_t for
grub_strncasecmp
2009-01-23 Daniel Mierswa impu...@impulze.org
* commands/search.c: caseless UUID comparing
signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] caseless uuid detection, fixed wrong behaviour for strncasecmp, added strcasecmp
Am 01/21/09 18:30, Pavel Roskin schrieb:
The patch looks good to me. I would split changes to commands/search.c
into a separate commit.
Please provide ChangeLog entries for the patches.
I will comply. Thanks for your quick response.
--
Mierswa, Daniel
If you still don't like it, that's ok: that's why I'm boss. I simply
know better than you do.
--- Linus Torvalds, comp.os.linux.advocacy, 1996/07/22
2009-01-23 Daniel Mierswa impu...@impulze.org
* kern/misc.c: add strcasecmp for consistency reasons, use grub_size_t
instead of int for strfuncs, fix strncasecmp return
values,
use the same algorithm in str*casecmp and str*cmp
* include/grub/misc.h: add str{,n}casecmp, use grub_size_t for
strncasecmp
2009-01-23 Daniel Mierswa impu...@impulze.org
* commands/search.c: caseless UUID comparing
Index: kern/misc.c
===
--- kern/misc.c (revision 1952)
+++ kern/misc.c (working copy)
@@ -194,7 +194,7 @@
while (*s1 *s2)
{
if (*s1 != *s2)
- return (int) *s1 - (int) *s2;
+break;
s1++;
s2++;
@@ -212,7 +212,7 @@
while (*s1 *s2 --n)
{
if (*s1 != *s2)
- return (int) *s1 - (int) *s2;
+break;
s1++;
s2++;
@@ -222,21 +222,36 @@
}
int
-grub_strncasecmp (const char *s1, const char *s2, int c)
+grub_strcasecmp (const char *s1, const char *s2)
{
- int p = 1;
+ while (*s1 *s2)
+{
+ if (grub_tolower (*s1) != grub_tolower (*s2))
+break;
+
+ s1++;
+ s2++;
+}
- while (grub_tolower (*s1) grub_tolower (*s2) p c)
+ return (int) grub_tolower (*s1) - (int) grub_tolower (*s2);
+}
+
+int
+grub_strncasecmp (const char *s1, const char *s2, grub_size_t n)
+{
+ if (n == 0)
+return 0;
+
+ while (*s1 *s2 --n)
{
if (grub_tolower (*s1) != grub_tolower (*s2))
- return (int) grub_tolower (*s1) - (int) grub_tolower (*s2);
+break;
s1++;
s2++;
- p++;
}
- return (int) *s1 - (int) *s2;
+ return (int) grub_tolower (*s1) - (int) grub_tolower (*s2);
}
char *
Index: include/grub/misc.h
===
--- include/grub/misc.h (revision 1952)
+++ include/grub/misc.h (working copy)
@@ -45,7 +45,8 @@
int EXPORT_FUNC(grub_memcmp) (const void *s1, const void *s2, grub_size_t n);
int EXPORT_FUNC(grub_strcmp) (const char *s1, const char *s2);
int EXPORT_FUNC(grub_strncmp) (const char *s1, const char *s2, grub_size_t n);
-int EXPORT_FUNC(grub_strncasecmp) (const char *s1, const char *s2, int c);
+int EXPORT_FUNC(grub_strcasecmp) (const char *s1, const char *s2);
+int EXPORT_FUNC(grub_strncasecmp) (const char *s1, const char *s2, grub_size_t
n);
char *EXPORT_FUNC(grub_strchr) (const char *s, int c);
char *EXPORT_FUNC(grub_strrchr) (const char *s, int c);
int EXPORT_FUNC(grub_strword) (const char *s, const char *w);
Index: commands/search.c
===
--- commands/search.c (revision 1952)
+++ commands/search.c (working copy)
@@ -115,7 +115,7 @@
(fs-uuid) (dev, uuid);
if (grub_errno == GRUB_ERR_NONE uuid)
{
- if (grub_strcmp (uuid, key) == 0)
+ if (grub_strcasecmp (uuid, key) == 0)
{
/* Found! */
count++;
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] caseless uuid detection, fixed wrong behaviour for strncasecmp, added strcasecmp
Hi list,
during testing I found that the UUID is checked case-dependend in
search.c, which is probably not wanted (I hope).
Also the grub_strncasecmp function returned (int) *s1 - (int) *s2 which
is wrong if you compare it to the C library strncasecmp.
I fixed that and used the same algorithm which is used in grub_strncmp
(Taking a grub_size_t instead of int and checked the decremented value
in the loop). I also added strcasecmp for consistency reasons which is
used by search.c now.
I'd appreciate your your replies.
--
Mierswa, Daniel
If you still don't like it, that's ok: that's why I'm boss. I simply
know better than you do.
--- Linus Torvalds, comp.os.linux.advocacy, 1996/07/22
Index: kern/misc.c
===
--- kern/misc.c (revision 1952)
+++ kern/misc.c (working copy)
@@ -194,7 +194,7 @@
while (*s1 *s2)
{
if (*s1 != *s2)
- return (int) *s1 - (int) *s2;
+break;
s1++;
s2++;
@@ -212,7 +212,7 @@
while (*s1 *s2 --n)
{
if (*s1 != *s2)
- return (int) *s1 - (int) *s2;
+break;
s1++;
s2++;
@@ -222,21 +222,36 @@
}
int
-grub_strncasecmp (const char *s1, const char *s2, int c)
+grub_strcasecmp (const char *s1, const char *s2)
{
- int p = 1;
+ while (*s1 *s2)
+{
+ if (grub_tolower (*s1) != grub_tolower (*s2))
+break;
+
+ s1++;
+ s2++;
+}
- while (grub_tolower (*s1) grub_tolower (*s2) p c)
+ return (int) grub_tolower (*s1) - (int) grub_tolower (*s2);
+}
+
+int
+grub_strncasecmp (const char *s1, const char *s2, grub_size_t n)
+{
+ if (n == 0)
+return 0;
+
+ while (*s1 *s2 --n)
{
if (grub_tolower (*s1) != grub_tolower (*s2))
- return (int) grub_tolower (*s1) - (int) grub_tolower (*s2);
+break;
s1++;
s2++;
- p++;
}
- return (int) *s1 - (int) *s2;
+ return (int) grub_tolower (*s1) - (int) grub_tolower (*s2);
}
char *
Index: include/grub/misc.h
===
--- include/grub/misc.h (revision 1952)
+++ include/grub/misc.h (working copy)
@@ -45,7 +45,8 @@
int EXPORT_FUNC(grub_memcmp) (const void *s1, const void *s2, grub_size_t n);
int EXPORT_FUNC(grub_strcmp) (const char *s1, const char *s2);
int EXPORT_FUNC(grub_strncmp) (const char *s1, const char *s2, grub_size_t n);
-int EXPORT_FUNC(grub_strncasecmp) (const char *s1, const char *s2, int c);
+int EXPORT_FUNC(grub_strcasecmp) (const char *s1, const char *s2);
+int EXPORT_FUNC(grub_strncasecmp) (const char *s1, const char *s2, grub_size_t
n);
char *EXPORT_FUNC(grub_strchr) (const char *s, int c);
char *EXPORT_FUNC(grub_strrchr) (const char *s, int c);
int EXPORT_FUNC(grub_strword) (const char *s, const char *w);
Index: commands/search.c
===
--- commands/search.c (revision 1952)
+++ commands/search.c (working copy)
@@ -115,7 +115,7 @@
(fs-uuid) (dev, uuid);
if (grub_errno == GRUB_ERR_NONE uuid)
{
- if (grub_strcmp (uuid, key) == 0)
+ if (grub_strcasecmp (uuid, key) == 0)
{
/* Found! */
count++;
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] caseless uuid detection, fixed wrong behaviour for strncasecmp, added strcasecmp
On Wed, 2009-01-21 at 13:08 +0100, Daniel Mierswa wrote: Hi list, during testing I found that the UUID is checked case-dependend in search.c, which is probably not wanted (I hope). Also the grub_strncasecmp function returned (int) *s1 - (int) *s2 which is wrong if you compare it to the C library strncasecmp. I agree, that's definitely wrong. Good catch! I fixed that and used the same algorithm which is used in grub_strncmp (Taking a grub_size_t instead of int and checked the decremented value in the loop). I also added strcasecmp for consistency reasons which is used by search.c now. I'd appreciate your your replies. The patch looks good to me. I would split changes to commands/search.c into a separate commit. Please provide ChangeLog entries for the patches. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
