[SSSD] [PATCH] UTIL: Fix access out of bound in parse_args

Pavel Reichl preichl at redhat.com
Wed Jun 25 08:35:47 UTC 2014


On Wed, 2014-06-25 at 09:54 +0200, Lukas Slebodnik wrote:
> On (24/06/14 18:07), Pavel Reichl wrote:
> >Hello Lukas,
> >
> >Patch looks OK to me, but I wonder if you could use \t for tabs? This
> >code is pain already:-).
> I strongly disagree.
> Unit test is not terible code. It's nice.

I didn't mean to insult your or anybody else's code. The UT and
especially parse_args are just difficult for me to comprehend. I thought
you also had hard time fixing parse_args, hadn't you?

> 
> Previously I didn't want to use '\t' instead of tabs because there is lof of
> backslashes anyway.
> 
> Let's compare
> diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c
> index 5ef0102..4f47305 100644
> --- a/src/tests/util-tests.c
> +++ b/src/tests/util-tests.c
> @@ -142,7 +142,7 @@ START_TEST(test_parse_args)
>      const char *parsed7[] = { "foo", "a", "f", NULL };
>      const char *parsed8[] = { "foo", "a\tg", NULL };
>      const char *parsed9[] = { "foo", NULL };
> -    const char *parsed10[] = { " ", "foo", "   ", "\\'", NULL };
> +    const char *parsed10[] = { " ", "foo", "\t", "\\'", NULL };
>      const char *parsed11[] = { "a", NULL };
>      struct pa_testcase tc[] = {
>          { "foo", parsed1 },
> @@ -151,14 +151,14 @@ START_TEST(test_parse_args)
>          { "foo a\\ c", parsed4 },
>          { "foo a d ", parsed5 },
>          { "foo   a   e   ", parsed6 },
> -        { "foo a               f       ", parsed7 },
> +        { "foo\ta\t \tf \t", parsed7 },
>          { "foo a\\\tg", parsed8 },
>          { "   foo  ", parsed9 },
> -        { "\\   foo \\  \\'  ", parsed10 },
> +        { "\\   foo \\\t \\'  ", parsed10 },
>          { "a", parsed11 },
>          { " ", NULL },
>          { "", NULL },
> -        { "      ", NULL },
> +        { " \t  ", NULL },
>          { NULL, NULL }
>      };
> 
> Each normal editor can show difference between tabs and spaces
> IMO, It looks to me like obfluscation after replacement.
> 
I personally prefer \t version, but I respect your opinion so unless
somebody else prefers explicit \t too, please keep you original version
as is.

> If you really want to do this change I can do it.
> I don't mind :-)
> 
> LS
> 
> PS: In future, try to avoid including html attachments into replies.
I'm sorry for that. Has this happen ever before or was this single
incident annoying so much you needed to stress it out?

> >> Summary > Report jvFA5F
> >> Bug Summary
> >> File:
> >> /sssd/bdir/../src/util/util.c
> >> Location:
> >> line 208, column 22
> >> Description:
> >> The left operand of '!=' is a
> >> garbage value
> >> Report Bug 
> >> Annotated Source Code
> >> 1
> >> /*
> >> 2
> >> Authors:
> >> 3
> >> Simo Sorce <ssorce at redhat.com>
> >> 4
> >> 
> >> 5
> >> Copyright (C) 2009 Red Hat
> >> 6
> >> 
> >> 7
> >> This program is free software; you
> >> can redistribute it and/or modify
> >> 8
> >> it under the terms of the GNU
> >> General Public License as published
> >> by
> >> 9
> >> the Free Software Foundation;
> >> either version 3 of the License, or
> >> 10
> >> (at your option) any later version.
> >> 11
> >> 
> >> 12
> >> This program is distributed in the
> >> hope that it will be useful,
> >> 13
> >> but WITHOUT ANY WARRANTY; without
> >> even the implied warranty of
> >> 14
> >> MERCHANTABILITY or FITNESS FOR A
> >> PARTICULAR PURPOSE. See the
> >> 15
> >> GNU General Public License for more
> >> details.
> >> 16
> >> 
> >> 17
> >> You should have received a copy of
> >> the GNU General Public License
> >> 18
> >> along with this program. If not,
> >> see <http://www.gnu.org/licenses/>.
> >> 19
> >> */
> >> 20
> 
> //2160 lines removed.
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel





More information about the sssd-devel mailing list