[lttng-dev] Regarding Bug #633 - utils_parse_size_suffix suffers from several problems

[Sandeep K Chaudhary]

Hi David,

Sorry for the delay in my response. I was away from school.

Here is the diff for changes made to the unit test. Please have a look.

index 3b9c68c..61124ff 100644
--- a/test_utils_parse_size_suffix.c.bkup
+++ b/test_utils_parse_size_suffix.c
@@ -37,6 +37,11 @@ static struct valid_test_input valid_tests_inputs[] = {
                { "0", 0 },
                { "1234", 1234 },
                { "0x400", 1024 },
+               { "0X400", 1024 },
+               { "0x40a", 1034 },
+               { "0X40b", 1035 },
+               { "0x40e", 1038 },
+               { "0X40f", 1039 },
                { "0300", 192 },
                { "16k", 16384 },
                { "128K", 131072 },
@@ -47,7 +52,7 @@ static struct valid_test_input valid_tests_inputs[] = {
 static const int num_valid_tests = sizeof(valid_tests_inputs) /
sizeof(valid_tests_inputs[0]);

 /* Invalid test cases */
-static char *invalid_tests_inputs[] = { "", "-1", "k",
"4611686018427387904G" };
+static char *invalid_tests_inputs[] = { "", "-1", "k", "08", "09",
"4611686018427387904G" };
 static const int num_invalid_tests = sizeof(invalid_tests_inputs) /
sizeof(invalid_tests_inputs[0]);

 static void test_utils_parse_size_suffix(void)

Thanks and regards,
Sandeep.




On Thu, Apr 3, 2014 at 8:06 AM, David Goulet <dgoulet at efficios.com> wrote:

> On 30 Mar (21:57:30), Sandeep K Chaudhary wrote:
> > Hi David,
> >
> > Add the unit tests for the changes. Please have a look at the following
> > diff for the changes in unit test file.
> >
> > 40,44d39
> > <               { "0X400", 1024 },
> > <               { "0x40a", 1034 },
> > <               { "0X40b", 1035 },
> > <               { "0x40e", 1038 },
> > <               { "0X40f", 1039 },
> > 55c50
> > < static char *invalid_tests_inputs[] = { "", "-1", "k", "08", "09",
> > "4611686018427387904G" };
> > ---
> > > static char *invalid_tests_inputs[] = { "", "-1", "k",
> > "4611686018427387904G" };
>
> Can you attach a diff patch to the issue please?
>
> Use "git diff" since the default "diff" command output is a bit
> unreadable for my passionate git eye :).
>
> Thanks!
> David
>
> >
> >
> > Thanks and regards,
> > Sandeep.
> >
> >
> > On Tue, Mar 25, 2014 at 7:22 AM, David Goulet <dgoulet at efficios.com>
> wrote:
> >
> > > On 21 Mar (00:22:57), Sandeep K Chaudhary wrote:
> > > > Hi guys,
> > > >
> > > > I checked the fix suggested in the bug description and it is correct.
> > > >
> > > > Changing
> > > >
> > > > ret = regcomp(&regex,
> > > "^\\(0x\\)\\{0,1\\}[0-9][0-9]*\\([kKMG]\\{0,1\\}\\)$", 0);
> > > >
> > > > to
> > > >
> > > > ret = regcomp(&regex,
> > > > "^(((0x|0X)[0-9A-Fa-f]+)|(0[0-7]*)|([1-9][0-9]*))[kKMG]?$",
> > > REG_EXTENDED);
> > > >
> > > > will take care of the following three problems that exist in the
> first
> > > regex
> > > >
> > > >
> > > >    - It accepts a leading 0 (without a following x or X) to
> represent an
> > > >    octal value, but simultaneously accepts the digits 8 and 9 in said
> > > octal
> > > >    value (this yields -1).
> > > >    - It does not support the leading 0X hexadecimal indicator but
> does
> > > >    support a leading 0x (see following).
> > > >    - It accepts a leading 0x to represent an hexadecimal value but
> > > rejects
> > > >    the a..f and A..F digits in said value.
> > > >
> > > >
> > > > Please let me know your views, and then I can submit a patch for
> this.
> > >
> > > Unfortunately I don't speak regex well enough to tell you if this is
> > > absolutely correct but what you should do with your patch is add unit
> > > test(s) that fix the three problems you list here.
> > >
> > > Most probably in:
> > >
> > >         tests/unit/test_utils_parse_size_suffix.c
> > >
> > > Cheers!
> > > David
> > >
> > > >
> > > > Thanks and regards,
> > > > Sandeep.
> > >
> > > > _______________________________________________
> > > > lttng-dev mailing list
> > > > lttng-dev at lists.lttng.org
> > > > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> > >
> > >
> >
> >
> > --
> > Thanks and regards,
> > Sandeep K Chaudhary.
>



-- 
Thanks and regards,
Sandeep K Chaudhary.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20140408/f869e6c6/attachment-0001.html>