Source-Changes-D archive

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

Re: CVS commit: src/sys/uvm



On Apr 18,  2:14pm, Christoph_Egger%gmx.de@localhost (Christoph Egger) wrote:
-- Subject: Re: CVS commit: src/sys/uvm

| Christos Zoulas wrote:
| > On Apr 18,  1:46pm, Christoph_Egger%gmx.de@localhost (Christoph Egger) 
wrote:
| > -- Subject: Re: CVS commit: src/sys/uvm
| > 
| > | This would imply to change the flags arguments of the pmap API
| > | from int to u_int to avoid troubles with checking flag bits.
| > 
| > As it should have been in the first place, so change it.
| 
| May I propose to introduce a MI flags_t for this purpose?
| 
| I would define it as
| 
| typedef u_int flags_t;
| 
| If this is fine, is sys/sys/types.h the right place ?
| If yes, should it be available in _KERNEL, in _NETBSD_SOURCE or
| generally ?

I am not sure I'd go that far but this is more for a discussion in tech-kern. 
The reason I am saying this is that we have used different width flags
depending on the number of bits needed or the number of bytes that are
available in a struct. It is also nice to explicitly know the number of
bits in the flags argument (which is more important for long than int I guess
since we don't have ILP64 systems).

There is also the issue of namespace pollution, so defining flags_t and
using it is problematic because it would have to be namespace protected.
For those reasons I say make it u_int for now, and start a discussion in
tech-kern.

| > 
| > | Alternatively, code like this
| > | 
| > |     if (flags & FLAG1)
| > |         do_something();
| > | 
| > | must be converted to
| > | 
| > |     if ((flags & FLAG1) == FLAG1)
| > |         do_something();
| > 
| > This is overkill.
| 
| and error-prone as it is very likely to be
| forgotten.
| 
| Just wanted to mention the alternative for completeness.

Yes, that is right, I agree.

christos


Home | Main Index | Thread Index | Old Index