Dan Kenigsberg wrote:> Hi, > > Please pardon my newbieness; I'm trying to understand what's what in ovirt (did > not get very far yet), and stumbled upon this:No problem, happy to help...> > commit f393d38599283676fbd33c81783fd7164faa4251 > Author: Dan Kenigsberg <danken at qumranet.com> > Date: Tue Oct 14 12:14:03 2008 +0200 > > ovirt-listen-awake: what's the point of streq? > > diff --git a/ovirt-listen-awake/ovirt-listen-awake.c b/ovirt-listen-awake/ovirt-listen-awake.c > index 7ecd0a7..b00a54d 100644 > --- a/ovirt-listen-awake/ovirt-listen-awake.c > +++ b/ovirt-listen-awake/ovirt-listen-awake.c > @@ -37,14 +37,7 @@ > > static int streq(char *first, char *second) > { > - int first_len, second_len; > - > - first_len = strlen(first); > - second_len = strlen(second); > - > - if (first_len == second_len && strncmp(first, second, first_len) == 0) > - return 1; > - return 0; > + return !strcmp(first, second); > }Heh. Well, the idea was that I wanted to be a little more defensive, so that "IDENTIFY" != "IDENTIFYFOO" (or whatever). I could have sworn I've run into bugs doing that in the past, but a quick test shows that strcmp() properly returns failure in that case. Ah, now I remember. I was taught always to use strncmp(), and just doing strncmp(first, second, strlen(first)) runs into the bug I was trying to avoid. In any case, not a big deal either way; it's not performance critical in the least, but I guess your version is a little easier to understand. If we are going to do it, we can just remove the streq function entirely, since it doesn't really serve any purpose then. By the way, you can send this stuff to ovirt-devel; I've added that as a CC here. -- Chris Lalancette
On Wed, Oct 15, 2008 at 01:14:29PM +0200, Chris Lalancette wrote:> Dan Kenigsberg wrote: > > Hi, > > > > Please pardon my newbieness; I'm trying to understand what's what in ovirt (did > > not get very far yet), and stumbled upon this: > > No problem, happy to help...Thanks, I will bother you again :-)> > > > > commit f393d38599283676fbd33c81783fd7164faa4251 > > Author: Dan Kenigsberg <danken at qumranet.com> > > Date: Tue Oct 14 12:14:03 2008 +0200 > > > > ovirt-listen-awake: what's the point of streq? > > > > diff --git a/ovirt-listen-awake/ovirt-listen-awake.c b/ovirt-listen-awake/ovirt-listen-awake.c > > index 7ecd0a7..b00a54d 100644 > > --- a/ovirt-listen-awake/ovirt-listen-awake.c > > +++ b/ovirt-listen-awake/ovirt-listen-awake.c > > @@ -37,14 +37,7 @@ > > > > static int streq(char *first, char *second) > > { > > - int first_len, second_len; > > - > > - first_len = strlen(first); > > - second_len = strlen(second); > > - > > - if (first_len == second_len && strncmp(first, second, first_len) == 0) > > - return 1; > > - return 0; > > + return !strcmp(first, second); > > } > > Heh. Well, the idea was that I wanted to be a little more defensive, so that > "IDENTIFY" != "IDENTIFYFOO" (or whatever). I could have sworn I've run into > bugs doing that in the past, but a quick test shows that strcmp() properly > returns failure in that case. > > Ah, now I remember. I was taught always to use strncmp(), and just doing > strncmp(first, second, strlen(first)) runs into the bug I was trying to avoid.My point is that your first_len = strlen(first);second_len = strlen(second);strncmp(first, second, first_len) runs into that bug, too...> In any case, not a big deal either way; it's not performance critical in the > least, but I guess your version is a little easier to understand. If we are > going to do it, we can just remove the streq function entirely, since it doesn't > really serve any purpose then. > > By the way, you can send this stuff to ovirt-devel; I've added that as a CC here.I was trying to keep my general disorientation lowkey, but hey, we're one happy company now... Dan.
Dan Kenigsberg wrote:>>> @@ -37,14 +37,7 @@ >>> >>> static int streq(char *first, char *second) >>> { >>> - int first_len, second_len; >>> - >>> - first_len = strlen(first); >>> - second_len = strlen(second); >>> - >>> - if (first_len == second_len && strncmp(first, second, first_len) == 0) >>> - return 1; >>> - return 0; >>> + return !strcmp(first, second); >>> } >> Heh. Well, the idea was that I wanted to be a little more defensive, so that >> "IDENTIFY" != "IDENTIFYFOO" (or whatever). I could have sworn I've run into >> bugs doing that in the past, but a quick test shows that strcmp() properly >> returns failure in that case. >> >> Ah, now I remember. I was taught always to use strncmp(), and just doing >> strncmp(first, second, strlen(first)) runs into the bug I was trying to avoid. > > My point is that your > first_len = strlen(first);second_len = strlen(second);strncmp(first, second, first_len) > runs into that bug, too...Ah, but it doesn't, because of the first_len == second_len guard. -- Chris Lalancette