Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/usr.bin/gzip



    Date:        Tue, 12 Jun 2018 19:52:43 +0200
    From:        Kamil Rytarowski <n54%gmx.com@localhost>
    Message-ID:  <90519caf-289c-b4d3-7ebc-d14d4efc9486%gmx.com@localhost>

  | I will try to meet the expectations in commit messages for *San fixes.

That would be good (and not just those,  unless the board required it, which
I doubt is likely, there's no need for the "Sponsored by" stuff almost 
anywhere, maybe just in those monthly reports you send).

  | I will try to also dig down why some programs needs in bit mask the sign
  | bit in unsigned integer instead of quickly swapping the type to unsigned.

It is probably important to verify that making the vars unsigned will not break
anything (normally for vars using bit fields and masking it will not) as 
breaking working code in the name of avoiding technical unspecified behaviour
is not really a good idea.

Fixing it is a good idea provided that the fixed version works (which it will
for the cases you have changed that I have looked at I believe) but it
is important that it really be a fix.

  | There are just many reports so I expected to land patches quickly, just
  | please be aware that presenting more deep investigation in a commit
  | message will take longer.

That isn't needed either - if a program is doing something that is undefined
all the commit message needs to say is something like

"Avoid unspecified behaviour, found by XXXsan"

That's it.   If someone wants to see what the unepcified behaviour is, then
they can check what lines were changed, if they can't work out why that code
was not correct, they can ask (but I dount that is going to happen often, most
people here recognise what C requires in this area - even if it is a bit 
stupid (IMO) and might not really be required for posix systems which require
that int types be 2's complement, unlike what C wants to be able to run on.)

kre



Home | Main Index | Thread Index | Old Index