Re: svn commit: r312119 - head/sys/kern

2017-01-15 Thread Ngie Cooper (yaneurabeya)

> On Jan 14, 2017, at 22:49, Bruce Evans  wrote:
> 
> On Sat, 14 Jan 2017, Ngie Cooper wrote:
> 
>> Log:
>> encode_long, encode_timeval: mechanically replace `exp` with `exponent`
>> 
>> This helps fix a -Wshadow issue with exp(3) with tests/sys/acct/acct_test,
>> which include math.h, which in turn defines exp(3)
> 
> But kern_acct.c doesn't include math.h.
> 
> This messes up the kernel sources to simplify abusing them in tests.
> 
> The bug was only in the sed script in the makefile that translates
> kern_acct.c to convert.c.  It converts 'log(' to 'syslog(', but is missing
> conversion of the exp identifier to sysexp.
> 
>> ==
>> --- head/sys/kern/kern_acct.cSat Jan 14 05:02:53 2017
>> (r312118)
>> +++ head/sys/kern/kern_acct.cSat Jan 14 05:06:14 2017
>> (r312119)
>> @@ -469,8 +469,8 @@ static uint32_t
>> encode_timeval(struct timeval tv)
>> {
>>  int log2_s;
>> -int val, exp;   /* Unnormalized value and exponent */
>> -int norm_exp;   /* Normalized exponent */
>> +int val, exponent;  /* Unnormalized value and exponent */
>> +int norm_exponent;  /* Normalized exponent */
>>  int shift;
>> 
>>  /*
> 
> Now the bug is also bad style in the kernel sources.  The regexp was too
> simple and munged norm_exp too, but not the exp's in comments.  The
> comments are more banal than before now that they don't even expand 'exp'
> but just echo 'exponent'.
> 
>> ...
>> -return (((FLT_MAX_EXP - 1 + exp + norm_exp) << (FLT_MANT_DIG - 1)) |
>> +return (((FLT_MAX_EXP - 1 + exponent + norm_exponent) << (FLT_MANT_DIG 
>> - 1)) |
>>  ((shift > 0 ? val << shift : val >> -shift) & MANT_MASK));
> 
> Here the expansion also broke the formatting.
> 
> The details of the abuse in the test program are that acct_test.c includes
> math.h and then includes then convert.c which is nearly a copy of the kernel
> source file.  This takes clean include files and not enabling warnings
> about redundant declarations to have a chance of working.
> 
> I use a similar hack to test libm, and didn't have to mess up the sources
> too much to make the translation not too hard.  Files have to be copied
> just to make the include paths manageable, and to compile them all with
> the same CFLAGS since this is a performance test.  The most complicated
> parts are to avoid library functions because they might not match the
> sources or were compiled with different CFLAGS.  The sources are not well
> organized well enough for my preferred method of "cc ${CLAGS} *.c" to work.

Thank you for the concerns. I’ve modified the approach based on your 
recommendations above and submitted it as r312216.
-Ngie


signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: svn commit: r312119 - head/sys/kern

2017-01-14 Thread Bruce Evans

On Sat, 14 Jan 2017, Ngie Cooper wrote:


Log:
 encode_long, encode_timeval: mechanically replace `exp` with `exponent`

 This helps fix a -Wshadow issue with exp(3) with tests/sys/acct/acct_test,
 which include math.h, which in turn defines exp(3)


But kern_acct.c doesn't include math.h.

This messes up the kernel sources to simplify abusing them in tests.

The bug was only in the sed script in the makefile that translates
kern_acct.c to convert.c.  It converts 'log(' to 'syslog(', but is missing
conversion of the exp identifier to sysexp.


==
--- head/sys/kern/kern_acct.c   Sat Jan 14 05:02:53 2017(r312118)
+++ head/sys/kern/kern_acct.c   Sat Jan 14 05:06:14 2017(r312119)
@@ -469,8 +469,8 @@ static uint32_t
encode_timeval(struct timeval tv)
{
int log2_s;
-   int val, exp;   /* Unnormalized value and exponent */
-   int norm_exp;   /* Normalized exponent */
+   int val, exponent;  /* Unnormalized value and exponent */
+   int norm_exponent;  /* Normalized exponent */
int shift;

/*


Now the bug is also bad style in the kernel sources.  The regexp was too
simple and munged norm_exp too, but not the exp's in comments.  The
comments are more banal than before now that they don't even expand 'exp'
but just echo 'exponent'.


...
-   return (((FLT_MAX_EXP - 1 + exp + norm_exp) << (FLT_MANT_DIG - 1)) |
+   return (((FLT_MAX_EXP - 1 + exponent + norm_exponent) << (FLT_MANT_DIG 
- 1)) |
((shift > 0 ? val << shift : val >> -shift) & MANT_MASK));


Here the expansion also broke the formatting.

The details of the abuse in the test program are that acct_test.c includes
math.h and then includes then convert.c which is nearly a copy of the kernel
source file.  This takes clean include files and not enabling warnings
about redundant declarations to have a chance of working.

I use a similar hack to test libm, and didn't have to mess up the sources
too much to make the translation not too hard.  Files have to be copied
just to make the include paths manageable, and to compile them all with
the same CFLAGS since this is a performance test.  The most complicated
parts are to avoid library functions because they might not match the
sources or were compiled with different CFLAGS.  The sources are not well
organized well enough for my preferred method of "cc ${CLAGS} *.c" to work.

Bruce
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r312119 - head/sys/kern

2017-01-13 Thread Ngie Cooper
Author: ngie
Date: Sat Jan 14 05:06:14 2017
New Revision: 312119
URL: https://svnweb.freebsd.org/changeset/base/312119

Log:
  encode_long, encode_timeval: mechanically replace `exp` with `exponent`
  
  This helps fix a -Wshadow issue with exp(3) with tests/sys/acct/acct_test,
  which include math.h, which in turn defines exp(3)
  
  MFC after:2 weeks
  Tested with:  clang, gcc 4.2.1, gcc 4.9
  Sponsored by: Dell EMC Isilon

Modified:
  head/sys/kern/kern_acct.c

Modified: head/sys/kern/kern_acct.c
==
--- head/sys/kern/kern_acct.c   Sat Jan 14 05:02:53 2017(r312118)
+++ head/sys/kern/kern_acct.c   Sat Jan 14 05:06:14 2017(r312119)
@@ -469,8 +469,8 @@ static uint32_t
 encode_timeval(struct timeval tv)
 {
int log2_s;
-   int val, exp;   /* Unnormalized value and exponent */
-   int norm_exp;   /* Normalized exponent */
+   int val, exponent;  /* Unnormalized value and exponent */
+   int norm_exponent;  /* Normalized exponent */
int shift;
 
/*
@@ -481,7 +481,7 @@ encode_timeval(struct timeval tv)
if (tv.tv_sec == 0) {
if (tv.tv_usec == 0)
return (0);
-   exp = 0;
+   exponent = 0;
val = tv.tv_usec;
} else {
/*
@@ -490,24 +490,24 @@ encode_timeval(struct timeval tv)
 */
log2_s = fls(tv.tv_sec) - 1;
if (log2_s + LOG2_1M < CALC_BITS) {
-   exp = 0;
+   exponent = 0;
val = 100 * tv.tv_sec + tv.tv_usec;
} else {
-   exp = log2_s + LOG2_1M - CALC_BITS;
+   exponent = log2_s + LOG2_1M - CALC_BITS;
val = (unsigned int)(((uint64_t)100 * tv.tv_sec +
-   tv.tv_usec) >> exp);
+   tv.tv_usec) >> exponent);
}
}
/* Now normalize and pack the value into an IEEE-754 float. */
-   norm_exp = fls(val) - 1;
-   shift = FLT_MANT_DIG - norm_exp - 1;
+   norm_exponent = fls(val) - 1;
+   shift = FLT_MANT_DIG - norm_exponent - 1;
 #ifdef ACCT_DEBUG
printf("val=%d exp=%d shift=%d log2(val)=%d\n",
-   val, exp, shift, norm_exp);
-   printf("exp=%x mant=%x\n", FLT_MAX_EXP - 1 + exp + norm_exp,
+   val, exponent, shift, norm_exponent);
+   printf("exp=%x mant=%x\n", FLT_MAX_EXP - 1 + exponent + norm_exponent,
((shift > 0 ? (val << shift) : (val >> -shift)) & MANT_MASK));
 #endif
-   return (((FLT_MAX_EXP - 1 + exp + norm_exp) << (FLT_MANT_DIG - 1)) |
+   return (((FLT_MAX_EXP - 1 + exponent + norm_exponent) << (FLT_MANT_DIG 
- 1)) |
((shift > 0 ? val << shift : val >> -shift) & MANT_MASK));
 }
 
@@ -518,7 +518,7 @@ encode_timeval(struct timeval tv)
 static uint32_t
 encode_long(long val)
 {
-   int norm_exp;   /* Normalized exponent */
+   int norm_exponent;  /* Normalized exponent */
int shift;
 
if (val == 0)
@@ -529,15 +529,15 @@ encode_long(long val)
val);
val = LONG_MAX;
}
-   norm_exp = fls(val) - 1;
-   shift = FLT_MANT_DIG - norm_exp - 1;
+   norm_exponent = fls(val) - 1;
+   shift = FLT_MANT_DIG - norm_exponent - 1;
 #ifdef ACCT_DEBUG
printf("val=%d shift=%d log2(val)=%d\n",
-   val, shift, norm_exp);
-   printf("exp=%x mant=%x\n", FLT_MAX_EXP - 1 + exp + norm_exp,
+   val, shift, norm_exponent);
+   printf("exp=%x mant=%x\n", FLT_MAX_EXP - 1 + exp + norm_exponent,
((shift > 0 ? (val << shift) : (val >> -shift)) & MANT_MASK));
 #endif
-   return (((FLT_MAX_EXP - 1 + norm_exp) << (FLT_MANT_DIG - 1)) |
+   return (((FLT_MAX_EXP - 1 + norm_exponent) << (FLT_MANT_DIG - 1)) |
((shift > 0 ? val << shift : val >> -shift) & MANT_MASK));
 }
 
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"