Re: CVS commit: src/usr.bin/xlint/lint1

2016-07-21 Thread Christos Zoulas
In article <20160721050113.ga1...@netbsd.org>,
David Holland   wrote:
>On Wed, Jul 20, 2016 at 02:14:12PM -0400, Christos Zoulas wrote:
> > Modified Files:
> > src/usr.bin/xlint/lint1: cgram.y
> > 
> > Log Message:
> > accept attributes in param decls
>
>Parsing gcc attributes "correctly" (as in, where gcc accepts them) is
>a trainwreck. Do we really want to bother trying?

It is actually pretty simple now; I am just discovering new places :-)

christos



Re: CVS commit: src/sys/uvm

2016-07-21 Thread Maxime Villard

Le 20/07/2016 à 20:07, matthew green a écrit :

"Maxime Villard" writes:

Module Name:src
Committed By:   maxv
Date:   Wed Jul 20 12:38:44 UTC 2016

Modified Files:
src/sys/uvm: uvm_extern.h uvm_km.c

Log Message:
Introduce uvm_km_protect.


can you bring this up on tech-kern and discuss what it is for?
this commit message is entirely useless today, let alone in the
future when someone tries to understand what is happening here.



[1], and then [2]. It's just a simple function to mprotect an area allocated
with uvm_km_alloc. It could also be used here [3].


[1] http://mail-index.netbsd.org/source-changes/2016/07/20/msg076249.html
[2] http://mail-index.netbsd.org/source-changes/2016/07/20/msg076250.html
[3] https://nxr.netbsd.org/xref/src/sys/dev/ic/sti.c#349


Re: CVS commit: src/sys/arch/x86/x86

2016-07-21 Thread Maxime Villard

Le 20/07/2016 à 07:07, Takashi YAMAMOTO a écrit :



On Wed, Jul 20, 2016 at 3:54 AM, Maxime Villard > wrote:

Module Name:src
Committed By:   maxv
Date:   Tue Jul 19 18:54:45 UTC 2016

Modified Files:
src/sys/arch/x86/x86: pmap.c

Log Message:
This loop makes no sense at all.


- can you provide more meaningful commit message?

- why don't you remove it then?



Look at the loop. It does not do anything. The bug was introduced six years
ago in this commit [1], and there is clearly a mistake: the guy just added
a loop around the code, but now the 'continue' refers to that new loop and
not the underlying one.

I could have fixed it to keep the original behavior, but in fact, I don't
even see why this hack is relevant. This hack is supposed to make sure we
never write-protect the PTE space, but as far as I can tell, the userland
space is below it, and the kernel space is above it. So if the kernel or
userland ever tries to write-protect this area, UVM will figure out it is
not included in the space and will reject the request.

In all cases, even if there were a way for someone to write-protect the PTE
space, this hack would still be wrong: some pages in the range wouldn't be
protected, and this is something the caller may not handle properly. And
even beyond that, if write-protecting the PTE space were possible, it would
be a huge flow in the design.

So all this makes absolutely no sense at all. I didn't want to spend too
much time on it, so I just added an XXX, for the next passer-by. I guess the
best thing to do would be turning the 'continue' to a 'panic'; or just
removing the loop completely, since the KASSERT below does mostly the same
thing.

[1] 
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/arch/x86/x86/pmap.c?only_with_tag=MAIN#rev1.112