Fixes for commit 3c49b23

Fixes for commit 3c49b23

[J William Piggott]

So, there are some issues with commit 3c49b23

* it contains multi-byte characters (which is what drew me to reading it).
* it truncates one very important piece of the formula: ". . . (mod 7)."
* it explains the values for 'e', but there is no 'e' in the code.
* it doesn't include a row resolving 'e' in the table, so it is not
   obvious what it relates to in the code.
* without citing it as an external reference, the comment language is confusing.

Anyway, I took a shot at trying to write a comment:

	 /*
	 * The magic constants in the reform[] array are, in a simplified
	 * sense, the remaining days after slicing into one week periods the total
	 * days from the beginning of the year to the target month. That is,
	 * weeks + reform[] days gets us to the target month. The exception is,
	 * that for the months past February 'DOY - 1' must be used.
	 *
	 *   DoY (Day of Year): total days to the target month
	 *
	 *   Month            1  2  3  4   5   6   7   8   9  10  11  12
	 *   DoY              0 31 59 90 120 151 181 212 243 273 304 334
	 *   DoY % 7          0  3
	 *   DoY - 1 % 7      - --  2  5   0   3   5   1   4   6   2   4
	 *       reform[] = { 0, 3, 2, 5,  0,  3,  5,  1,  4,  6,  2,  4 };
	 *
	 *  Note: these calculations are for non leap years.
	 */

I cannot send this as a patch, because my system mangles the multi-byte
characters so it won't apply cleanly.

On a somewhat related note, multi-byte characters are also making it
into the commit log; it's not a big deal I suppose, but it sure makes a
mess for systems that don't support them.

Re: Fixes for commit 3c49b23

[Karel Zak]

On Sun, Dec 17, 2017 at 07:47:49PM -0500, J William Piggott wrote:
> 
> So, there are some issues with commit 3c49b23
> 
> * it contains multi-byte characters (which is what drew me to reading it).

Ah, it was copy & past from some white papers. Sorry.

> * it truncates one very important piece of the formula: ". . . (mod 7)."
> * it explains the values for 'e', but there is no 'e' in the code.
> * it doesn't include a row resolving 'e' in the table, so it is not
>    obvious what it relates to in the code.
> * without citing it as an external reference, the comment language is confusing.
> 
> Anyway, I took a shot at trying to write a comment:

Thanks!

> 
> 	 /*
> 	 * The magic constants in the reform[] array are, in a simplified
> 	 * sense, the remaining days after slicing into one week periods the total
> 	 * days from the beginning of the year to the target month. That is,
> 	 * weeks + reform[] days gets us to the target month. The exception is,
> 	 * that for the months past February 'DOY - 1' must be used.
> 	 *
> 	 *   DoY (Day of Year): total days to the target month
> 	 *
> 	 *   Month            1  2  3  4   5   6   7   8   9  10  11  12
> 	 *   DoY              0 31 59 90 120 151 181 212 243 273 304 334
> 	 *   DoY % 7          0  3
> 	 *   DoY - 1 % 7      - --  2  5   0   3   5   1   4   6   2   4
> 	 *       reform[] = { 0, 3, 2, 5,  0,  3,  5,  1,  4,  6,  2,  4 };
> 	 *
> 	 *  Note: these calculations are for non leap years.
> 	 */
> 
> I cannot send this as a patch, because my system mangles the multi-byte
> characters so it won't apply cleanly.
> 
> On a somewhat related note, multi-byte characters are also making it
> into the commit log; it's not a big deal I suppose, but it sure makes a
> mess for systems that don't support them.

Well, it's too late, and I think commit messages is not so big issue :-)

Applied, thanks!

    Karel

> 
> 
> 
> 

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: Fixes for commit 3c49b23

[J William Piggott]

On 12/18/2017 06:49 AM, Karel Zak wrote:
> On Sun, Dec 17, 2017 at 07:47:49PM -0500, J William Piggott wrote:


>>
>> On a somewhat related note, multi-byte characters are also making it
>> into the commit log; it's not a big deal I suppose, but it sure makes a
>> mess for systems that don't support them.
> 
> Well, it's too late, and I think commit messages is not so big issue :-)

Ha, I wasn't suggesting amending the entire commit log ;). I was hoping perhaps
it could be avoided in future commits. It appears to mostly be caused by
copy/paste there also. For example:

 ---

commit b9e4ee42c285e73b36879a96b9cde43edb0f90c6
Author: Karel Zak <kzak@redhat.com>
Date:   Tue Nov 28 10:53:35 2017 +0100

    nsenter: fix compiler warning [-Wuninitialized]
    
    sys-utils/nsenter.c: In function â<80><98>is_same_namespaceâ<80><99>:
    sys-utils/nsenter.c:170:2: warning: â<80><98>b_inoâ<80><99> may be used uninitialized in this function [-Wuninitialized]
    sys-utils/nsenter.c:170:2: warning: â<80><98>a_inoâ<80><99> may be used uninitialized in this function [-Wuninitialized]
    
    Signed-off-by: Karel Zak <kzak@redhat.com>

 ---

Looks like copy/paste from a terminal. Maybe there is no easy way to avoid
that? Would setting LANG=C when running the command due it? Maybe even that is
too much hassle.

Anyway, as we've both said, it's not a big deal. It just looks ugly.



> 
> Applied, thanks!
> 
>     Karel
> 
>>
>>
>>
>>
> 

Re: Fixes for commit 3c49b23

[Karel Zak]

On Mon, Dec 18, 2017 at 01:02:50PM -0500, J William Piggott wrote:
> commit b9e4ee42c285e73b36879a96b9cde43edb0f90c6
> Author: Karel Zak <kzak@redhat.com>
> Date:   Tue Nov 28 10:53:35 2017 +0100
> 
>     nsenter: fix compiler warning [-Wuninitialized]
>     
>     sys-utils/nsenter.c: In function â<80><98>is_same_namespaceâ<80><99>:
>     sys-utils/nsenter.c:170:2: warning: â<80><98>b_inoâ<80><99> may be used uninitialized in this function [-Wuninitialized]
>     sys-utils/nsenter.c:170:2: warning: â<80><98>a_inoâ<80><99> may be used uninitialized in this function [-Wuninitialized]
>     
>     Signed-off-by: Karel Zak <kzak@redhat.com>
> 
>  ---
> 
> Looks like copy/paste from a terminal. Maybe there is no easy way to avoid
> that? Would setting LANG=C when running the command due it? Maybe even that is
> too much hassle.

Yes, it's copy & past from terminal. I'll try to avoid this in future.
Maybe we can use some git commit hook to see a warning, not sure how
difficult is to write such hook.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com