diff mbox series

[Openvpn-devel] Add %d, %u and %lu tests to test_argv unit tests.

Message ID 20180623121503.4080-1-gert@greenie.muc.de
State Changes Requested
Headers
Series [Openvpn-devel] Add %d, %u and %lu tests to test_argv unit tests. |

Commit Message

Gert Doering June 23, 2018, 2:15 a.m. UTC 
  Some basic integer tests to verify signed, unsigned and
long unsigned (2^33) printing.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 tests/unit_tests/openvpn/test_argv.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Gert Doering June 23, 2018, 7:31 a.m. UTC | #1 
Hi,

On Sat, Jun 23, 2018 at 02:15:03PM +0200, Gert Doering wrote:
> Some basic integer tests to verify signed, unsigned and
> long unsigned (2^33) printing.

Ditch that patch... this only works on 64bit systems.

What I wanted to see is "yes, it really does long math", but on systems
where sizeof(int) = sizeof(long) = 4, this will just not do the right
thing.

Do we care?  

Or shall I just have it print "1", so we know "%lu works and prints its 
argument nicely"?

gert
Selva Nair June 23, 2018, 8:08 a.m. UTC | #2 
Hi,

On Sat, Jun 23, 2018 at 1:31 PM, Gert Doering <gert@greenie.muc.de> wrote:

> Hi,
>
> On Sat, Jun 23, 2018 at 02:15:03PM +0200, Gert Doering wrote:
> > Some basic integer tests to verify signed, unsigned and
> > long unsigned (2^33) printing.
>
> Ditch that patch... this only works on 64bit systems.
>

Most 64 bit systems (excluding Windows)


>
> What I wanted to see is "yes, it really does long math", but on systems
> where sizeof(int) = sizeof(long) = 4, this will just not do the right
> thing.
>
> Do we care?
>
> Or shall I just have it print "1", so we know "%lu works and prints its
> argument nicely"?
>

That should be enough, but we could get fancy and use -1L and compare
the result against sprintf(str, "%lu", -1L)

Selva
<div dir="ltr">Hi,<br><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Jun 23, 2018 at 1:31 PM, Gert Doering <span dir="ltr">&lt;<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<span class=""><br>
On Sat, Jun 23, 2018 at 02:15:03PM +0200, Gert Doering wrote:<br>
&gt; Some basic integer tests to verify signed, unsigned and<br>
&gt; long unsigned (2^33) printing.<br>
<br>
</span>Ditch that patch... this only works on 64bit systems.<br></blockquote><div><br></div><div>Most 64 bit systems (excluding Windows)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
What I wanted to see is &quot;yes, it really does long math&quot;, but on systems<br>
where sizeof(int) = sizeof(long) = 4, this will just not do the right<br>
thing.<br>
<br>
Do we care?  <br>
<br>
Or shall I just have it print &quot;1&quot;, so we know &quot;%lu works and prints its <br>
argument nicely&quot;?<br></blockquote><div><br></div><div>That should be enough, but we could get fancy and use -1L and compare</div><div>the result against sprintf(str, &quot;%lu&quot;, -1L)</div><div><br></div><div>Selva</div><div><br></div></div></div></div>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Antonio Quartulli June 23, 2018, 8:12 a.m. UTC | #3 
Hi,

On 24/06/18 02:08, Selva Nair wrote:
>> Or shall I just have it print "1", so we know "%lu works and prints its
>> argument nicely"?
>>
> 
> That should be enough, but we could get fancy and use -1L and compare
> the result against sprintf(str, "%lu", -1L)
> 

I just recommended something similar on IRC.
I think it would make sense to "validate" argv_printf* against sprintf.
Not just for %lu but also for the other formats. This way we know our
code is doing what sprintf would also do.



Cheers,
Gert Doering June 23, 2018, 8:16 a.m. UTC | #4 
Hi,

On Sun, Jun 24, 2018 at 02:12:29AM +0800, Antonio Quartulli wrote:
> > That should be enough, but we could get fancy and use -1L and compare
> > the result against sprintf(str, "%lu", -1L)
> 
> I just recommended something similar on IRC.
> I think it would make sense to "validate" argv_printf* against sprintf.
> Not just for %lu but also for the other formats. This way we know our
> code is doing what sprintf would also do.

I'll go for the "just 1" thing so we have something.  

If one of you feels bored, I'm happy to test something more fancy :-) -
but I do not feel particularily like "coming up with fancy tests for
argv.c" today.

gert
diff mbox series

Patch

diff --git a/tests/unit_tests/openvpn/test_argv.c b/tests/unit_tests/openvpn/test_argv.c
index 4a3ba559..0e7a4513 100644
--- a/tests/unit_tests/openvpn/test_argv.c
+++ b/tests/unit_tests/openvpn/test_argv.c
@@ -116,8 +116,12 @@  argv_str__multiple_argv__correct_output(void **state)
     argv_printf(&a, "%s%sc", PATH1, PATH2);
     argv_printf_cat(&a, "%s", PARAM1);
     argv_printf_cat(&a, "%s", PARAM2);
+    argv_printf_cat(&a, "%d", -1);
+    argv_printf_cat(&a, "%u", -1);
+    argv_printf_cat(&a, "%lu", (unsigned long) (1L<<33) );
     output = argv_str(&a, &gc, PA_BRACKET);
-    assert_string_equal(output, "[" PATH1 PATH2 "] [" PARAM1 "] [" PARAM2 "]");
+    assert_string_equal(output, "[" PATH1 PATH2 "] [" PARAM1 "] [" PARAM2 "]"
+				" [-1] [4294967295] [8589934592]");
 
     argv_reset(&a);
     gc_free(&gc);