[Git][NTPsec/ntpsec][master] binio: change get_lsb_ulong() to get_lsb_int32()

Gary E. Miller gitlab at mg.gitlab.com
Sat May 13 00:14:03 UTC 2017


Gary E. Miller pushed to branch master at NTPsec / ntpsec


Commits:
a88032ec by Gary E. Miller at 2017-05-12T17:07:07-07:00
binio: change get_lsb_ulong() to get_lsb_int32()

And add macro get_lsb_uint32().  Enable 2 commented out tests that
now pass.o
Since get_lsb_ulong() was sometimes used as an int_32_t and sometimes
as  a uint32_t this makes the code more explicit anf finesses different
CC doing different, and incompatible, things with the code.

It turns out that some, not all, CC will promote an (unsigned char)
to (int) when you left shift it.  This is required in C99 6.3.1.1.2.
But Solaris does not do that.  Then when the new int is sign extended
and ORed in an unsigned, bad things happen.

- - - - -


4 changed files:

- include/binio.h
- libparse/binio.c
- libparse/data_mbg.c
- tests/libparse/binio.c


Changes:

=====================================
include/binio.h
=====================================
--- a/include/binio.h
+++ b/include/binio.h
@@ -12,11 +12,12 @@
 
 long get_lsb_short (unsigned char **);
 void put_lsb_short (unsigned char **, long);
-unsigned long get_lsb_ulong (unsigned char **);
+int32_t get_lsb_int32 (unsigned char **);
 void put_lsb_long (unsigned char **, long);
 
 #define get_lsb_int16( _x_ )   ((int16_t) get_lsb_short( _x_ ))
 #define get_lsb_uint16( _x_ )  ((uint16_t) get_lsb_short( _x_ ))
+#define get_lsb_uint32( _x_ )  ((uint32_t) get_lsb_int32( _x_ ))
 
 long get_msb_short (unsigned char **);
 void put_msb_short (unsigned char **, long);


=====================================
libparse/binio.c
=====================================
--- a/libparse/binio.c
+++ b/libparse/binio.c
@@ -32,19 +32,19 @@ put_lsb_short(
   *((*bufpp)++) = (unsigned char) ((val >> 8) & 0xFF);
 }
 
-unsigned long
-get_lsb_ulong(
+int32_t
+get_lsb_int32(
 	unsigned char **bufpp
 	)
 {
-  long retval;
+  int32_t retval;
 
   retval  = *((*bufpp)++);
   retval |= *((*bufpp)++) << 8;
   retval |= *((*bufpp)++) << 16;
   retval |= *((*bufpp)++) << 24;
 
-  return (unsigned long)retval;
+  return retval;
 }
 
 void


=====================================
libparse/data_mbg.c
=====================================
--- a/libparse/data_mbg.c
+++ b/libparse/data_mbg.c
@@ -95,8 +95,8 @@ get_mbg_tgps(
 	)
 {
   tgpsp->wn = get_lsb_uint16(bufpp);
-  tgpsp->sec = get_lsb_ulong(bufpp);
-  tgpsp->tick = get_lsb_ulong(bufpp);
+  tgpsp->sec = get_lsb_uint32(bufpp);
+  tgpsp->tick = get_lsb_uint32(bufpp);
 }
 
 void
@@ -113,8 +113,8 @@ get_mbg_tm(
   tmp->hour = (int8_t)(*(*buffpp)++);
   tmp->min = (int8_t)(*(*buffpp)++);
   tmp->sec = (int8_t)(*(*buffpp)++);
-  tmp->frac = get_lsb_ulong(buffpp);
-  tmp->offs_from_utc = get_lsb_ulong(buffpp);
+  tmp->frac = get_lsb_int32(buffpp);
+  tmp->offs_from_utc = get_lsb_int32(buffpp);
   tmp->status = get_lsb_uint16(buffpp);
 }
 
@@ -156,8 +156,8 @@ get_mbg_tzdl(
 	TZDL *tzdlp
 	)
 {
-  tzdlp->offs = get_lsb_ulong(buffpp);
-  tzdlp->offs_dl = get_lsb_ulong(buffpp);
+  tzdlp->offs = get_lsb_int32(buffpp);
+  tzdlp->offs_dl = get_lsb_int32(buffpp);
   get_mbg_tm(buffpp, &tzdlp->tm_on);
   get_mbg_tm(buffpp, &tzdlp->tm_off);
   get_mbg_tzname(buffpp, (char *)tzdlp->name[0]);
@@ -173,7 +173,7 @@ get_mbg_antinfo(
   antinfop->status = get_lsb_int16(buffpp);
   get_mbg_tm(buffpp, &antinfop->tm_disconn);
   get_mbg_tm(buffpp, &antinfop->tm_reconn);
-  antinfop->delta_t = get_lsb_ulong(buffpp);
+  antinfop->delta_t = get_lsb_int32(buffpp);
 }
 
 static void
@@ -355,7 +355,7 @@ get_mbg_comparam(
 {
   size_t i;
 
-  comparamp->baud_rate = get_lsb_ulong(buffpp);
+  comparamp->baud_rate = get_lsb_int32(buffpp);
   for (i = 0; i < sizeof(comparamp->framing); i++)
     {
       comparamp->framing[i] = (char)(*(*buffpp)++);


=====================================
tests/libparse/binio.c
=====================================
--- a/tests/libparse/binio.c
+++ b/tests/libparse/binio.c
@@ -70,59 +70,58 @@ TEST(binio, get_lsb_short4) {
 }
 
 
-TEST(binio, get_lsb_ulong0) {
-        unsigned long ret;
+TEST(binio, get_lsb_int320) {
+        int32_t ret;
         unsigned char zero[4] = { 0, 0, 0, 0};
         unsigned char *bp = &zero[0];
 
-        ret = get_lsb_ulong( &bp);
+        ret = get_lsb_int32( &bp);
 
-        TEST_ASSERT_EQUAL_UINT64( 0, ret );
+        TEST_ASSERT_EQUAL_INT32( 0, ret );
 }
 
 
-TEST(binio, get_lsb_ulong1) {
-        unsigned long ret;
+TEST(binio, get_lsb_int321) {
+        int32_t ret;
         unsigned char zero[4] = { 1, 2, 3, 4};
         unsigned char *bp = &zero[0];
 
-        ret = get_lsb_ulong( &bp);
+        ret = get_lsb_int32( &bp);
 
-        TEST_ASSERT_EQUAL_HEX64( 0x04030201UL, ret );
+        TEST_ASSERT_EQUAL_HEX32( 0x04030201UL, ret );
 }
 
 
-TEST(binio, get_lsb_ulong2) {
-        unsigned long ret;
+TEST(binio, get_lsb_int322) {
+        int32_t ret;
         unsigned char zero[4] = { 4, 3, 2, 1};
         unsigned char *bp = &zero[0];
 
-        ret = get_lsb_ulong( &bp);
+        ret = get_lsb_int32( &bp);
 
-        TEST_ASSERT_EQUAL_HEX64( 0x01020304UL, ret );
+        TEST_ASSERT_EQUAL_HEX32( 0x01020304UL, ret );
 }
 
 
-TEST(binio, get_lsb_ulong3) {
-        unsigned long ret;
+TEST(binio, get_lsb_int323) {
+        int32_t ret;
         unsigned char zero[4] = { 0xff, 0xff, 0xff, 0xff};
         unsigned char *bp = &zero[0];
 
-        ret = get_lsb_ulong( &bp);
+        ret = get_lsb_int32( &bp);
 
-        printf("ulong: %lx\n", ret);
-        TEST_ASSERT_EQUAL_HEX64( 0x0FFFFFFFFUL, ret );
+        TEST_ASSERT_EQUAL_HEX32( 0x0FFFFFFFFUL, ret );
 }
 
 
-TEST(binio, get_lsb_ulong4) {
-        unsigned long ret;
+TEST(binio, get_lsb_int324) {
+        int32_t ret;
         unsigned char zero[4] = { 0, 0, 0, 0x80};
         unsigned char *bp = &zero[0];
 
-        ret = get_lsb_ulong( &bp);
+        ret = get_lsb_int32( &bp);
 
-        TEST_ASSERT_EQUAL_HEX64( 0x080000000UL, ret );
+        TEST_ASSERT_EQUAL_HEX32( 0x080000000UL, ret );
 }
 
 /* MSB tests */
@@ -240,13 +239,11 @@ TEST_GROUP_RUNNER(binio) {
         RUN_TEST_CASE(binio, get_lsb_short3);
         RUN_TEST_CASE(binio, get_lsb_short4);
 
-        RUN_TEST_CASE(binio, get_lsb_ulong0);
-        RUN_TEST_CASE(binio, get_lsb_ulong1);
-        RUN_TEST_CASE(binio, get_lsb_ulong2);
-        /* next two tests are good, but the code they test is bad
-        RUN_TEST_CASE(binio, get_lsb_ulong3);
-        RUN_TEST_CASE(binio, get_lsb_ulong4);
-        */
+        RUN_TEST_CASE(binio, get_lsb_int320);
+        RUN_TEST_CASE(binio, get_lsb_int321);
+        RUN_TEST_CASE(binio, get_lsb_int322);
+        RUN_TEST_CASE(binio, get_lsb_int323);
+        RUN_TEST_CASE(binio, get_lsb_int324);
 
         /* MSB tests */
         RUN_TEST_CASE(binio, get_msb_short0);



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/a88032ec1272a904eb3837a5875d44d7fe0e3abe

---
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/a88032ec1272a904eb3837a5875d44d7fe0e3abe
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20170513/a5ad2478/attachment.html>


More information about the vc mailing list