Style fixups for proc.h
Hi OK to commit the style fixups below? (They just make sure that all function prototypes have all arguments named, and that all names are protected). int foo(int bar); becomes int foo(int _bar); M Index: proc.h === RCS file: /home/ncvs/src/sys/sys/proc.h,v retrieving revision 1.291 diff -u -d -r1.291 proc.h @@ -860,101 +860,101 @@ extern int lastpid; -struct proc *pfind(pid_t); /* Find process by id. */ -struct pgrp *pgfind(pid_t);/* Find process group by id. */ -struct proc *zpfind(pid_t);/* Find zombie process by id. */ +struct proc *pfind(pid_t _pid);/* Find process by id. */ +struct pgrp *pgfind(pid_t _pid); /* Find process group by id. */ +struct proc *zpfind(pid_t _pid); /* Find zombie process by id. */ -void adjustrunqueue(struct thread *, int newpri); -void ast(struct trapframe *framep); +void adjustrunqueue(struct thread *_td, int _newpri); +void ast(struct trapframe *_framep); struct thread *choosethread(void); -intcr_cansignal(struct ucred *cred, struct proc *proc, int signum); -intenterpgrp(struct proc *p, pid_t pgid, struct pgrp *pgrp, struct session *sess); -intenterthispgrp(struct proc *p, struct pgrp *pgrp); -void faultin(struct proc *p); -void fixjobc(struct proc *p, struct pgrp *pgrp, int entering); -intfork1(struct thread *, int, int, struct proc **); -void fork_exit(void (*)(void *, struct trapframe *), void *, - struct trapframe *); -void fork_return(struct thread *, struct trapframe *); -intinferior(struct proc *p); -intleavepgrp(struct proc *p); +intcr_cansignal(struct ucred *_cred, struct proc *_p, int _signum); +intenterpgrp(struct proc *_p, pid_t _pgid, struct pgrp *pgrp, struct session +*_sess); +intenterthispgrp(struct proc *_p, struct pgrp *_pgrp); +void faultin(struct proc *_p); +void fixjobc(struct proc *_p, struct pgrp *_pgrp, int _entering); +intfork1(struct thread *_td, int, int, struct proc **_p); +void fork_exit(void (*_func)(void *_ptr, struct trapframe *_tf), void *_ptr, + struct trapframe *_tf); +void fork_return(struct thread *_td, struct trapframe *_tf); +intinferior(struct proc *_p); +intleavepgrp(struct proc *_p); void mi_switch(void); -intp_candebug(struct thread *td, struct proc *p); -intp_cansee(struct thread *td, struct proc *p); -intp_cansched(struct thread *td, struct proc *p); -intp_cansignal(struct thread *td, struct proc *p, int signum); -struct pargs *pargs_alloc(int len); -void pargs_drop(struct pargs *pa); -void pargs_free(struct pargs *pa); -void pargs_hold(struct pargs *pa); +intp_candebug(struct thread *_td, struct proc *_p); +intp_cansee(struct thread *_td, struct proc *_p); +intp_cansched(struct thread *_td, struct proc *_p); +intp_cansignal(struct thread *_td, struct proc *_p, int _signum); +struct pargs *pargs_alloc(int _len); +void pargs_drop(struct pargs *_pa); +void pargs_free(struct pargs *_pa); +void pargs_hold(struct pargs *_pa); void procinit(void); void threadinit(void); -void proc_linkup(struct proc *p, struct ksegrp *kg, - struct kse *ke, struct thread *td); -void proc_reparent(struct proc *child, struct proc *newparent); -intsecurelevel_ge(struct ucred *cr, int level); +void proc_linkup(struct proc *_p, struct ksegrp *_kg, + struct kse *_ke, struct thread *_td); +void proc_reparent(struct proc *_child, struct proc *_newparent); +intsecurelevel_ge(struct ucred *_cr, int _level); intsecurelevel_gt(struct ucred *cr, int level); -void setrunnable(struct thread *); -void setrunqueue(struct thread *); -void setsugid(struct proc *p); +void setrunnable(struct thread *_td); +void setrunqueue(struct thread *_td); +void setsugid(struct proc *_p); void sleepinit(void); -void stopevent(struct proc *, u_int, u_int); +void stopevent(struct proc *_p, u_int _arg1, u_int _arg2); void cpu_idle(void); void cpu_switch(void); void cpu_throw(void) __dead2; -void unsleep(struct thread *); -void userret(struct thread *, struct trapframe *, u_int); +void unsleep(struct thread *_td); +void userret(struct thread *_td, struct trapframe *_tf, u_int _arg); -void cpu_exit(struct thread *); -void cpu_sched_exit(struct thread *); -void exit1(struct thread *, int) __dead2; -void cpu_fork(struct thread *, struct proc *, struct thread *, int); -void cpu_set_fork_handler(struct thread *, void (*)(void *), void *); -void cpu_wait(struct proc *); +void cpu_exit(struct thread *_td); +void cpu_sched_exit(struct thread *_td); +void exit1(struct thread *_td, int _ret) __dead2; +void cpu_fork(struct thread *_td1, struct proc *_p, struct thread *_td2, int _ret); +void cpu_set_fork_handler(struct thread *_td, void (*_pfunc)(void *_arg), void +*_ptr); +void cpu_wait(struct proc *_p); /* New in KSE. */ struct ksegrp
Re: Style fixups for proc.h
I don't know about the protection with a '_'. It's not standard and usually the name matches that used in the actual function. It's certainly not part of style(9) that I've ever noticed and it's generally noy done that way.. is there a move to do this on all the other files? On Sat, 1 Feb 2003, Mark Murray wrote: Hi OK to commit the style fixups below? (They just make sure that all function prototypes have all arguments named, and that all names are protected). int foo(int bar); becomes int foo(int _bar); M Index: proc.h === RCS file: /home/ncvs/src/sys/sys/proc.h,v retrieving revision 1.291 diff -u -d -r1.291 proc.h @@ -860,101 +860,101 @@ extern int lastpid; -struct proc *pfind(pid_t); /* Find process by id. */ -struct pgrp *pgfind(pid_t);/* Find process group by id. */ -struct proc *zpfind(pid_t);/* Find zombie process by id. */ +struct proc *pfind(pid_t _pid);/* Find process by id. */ +struct pgrp *pgfind(pid_t _pid); /* Find process group by id. */ +struct proc *zpfind(pid_t _pid); /* Find zombie process by id. */ -void adjustrunqueue(struct thread *, int newpri); -void ast(struct trapframe *framep); +void adjustrunqueue(struct thread *_td, int _newpri); +void ast(struct trapframe *_framep); struct thread *choosethread(void); -int cr_cansignal(struct ucred *cred, struct proc *proc, int signum); -int enterpgrp(struct proc *p, pid_t pgid, struct pgrp *pgrp, struct session *sess); -int enterthispgrp(struct proc *p, struct pgrp *pgrp); -void faultin(struct proc *p); -void fixjobc(struct proc *p, struct pgrp *pgrp, int entering); -int fork1(struct thread *, int, int, struct proc **); -void fork_exit(void (*)(void *, struct trapframe *), void *, - struct trapframe *); -void fork_return(struct thread *, struct trapframe *); -int inferior(struct proc *p); -int leavepgrp(struct proc *p); +int cr_cansignal(struct ucred *_cred, struct proc *_p, int _signum); +int enterpgrp(struct proc *_p, pid_t _pgid, struct pgrp *pgrp, struct session *_sess); +int enterthispgrp(struct proc *_p, struct pgrp *_pgrp); +void faultin(struct proc *_p); +void fixjobc(struct proc *_p, struct pgrp *_pgrp, int _entering); +int fork1(struct thread *_td, int, int, struct proc **_p); +void fork_exit(void (*_func)(void *_ptr, struct trapframe *_tf), void *_ptr, + struct trapframe *_tf); +void fork_return(struct thread *_td, struct trapframe *_tf); +int inferior(struct proc *_p); +int leavepgrp(struct proc *_p); void mi_switch(void); -int p_candebug(struct thread *td, struct proc *p); -int p_cansee(struct thread *td, struct proc *p); -int p_cansched(struct thread *td, struct proc *p); -int p_cansignal(struct thread *td, struct proc *p, int signum); -struct pargs *pargs_alloc(int len); -void pargs_drop(struct pargs *pa); -void pargs_free(struct pargs *pa); -void pargs_hold(struct pargs *pa); +int p_candebug(struct thread *_td, struct proc *_p); +int p_cansee(struct thread *_td, struct proc *_p); +int p_cansched(struct thread *_td, struct proc *_p); +int p_cansignal(struct thread *_td, struct proc *_p, int _signum); +struct pargs *pargs_alloc(int _len); +void pargs_drop(struct pargs *_pa); +void pargs_free(struct pargs *_pa); +void pargs_hold(struct pargs *_pa); void procinit(void); void threadinit(void); -void proc_linkup(struct proc *p, struct ksegrp *kg, - struct kse *ke, struct thread *td); -void proc_reparent(struct proc *child, struct proc *newparent); -int securelevel_ge(struct ucred *cr, int level); +void proc_linkup(struct proc *_p, struct ksegrp *_kg, + struct kse *_ke, struct thread *_td); +void proc_reparent(struct proc *_child, struct proc *_newparent); +int securelevel_ge(struct ucred *_cr, int _level); int securelevel_gt(struct ucred *cr, int level); -void setrunnable(struct thread *); -void setrunqueue(struct thread *); -void setsugid(struct proc *p); +void setrunnable(struct thread *_td); +void setrunqueue(struct thread *_td); +void setsugid(struct proc *_p); void sleepinit(void); -void stopevent(struct proc *, u_int, u_int); +void stopevent(struct proc *_p, u_int _arg1, u_int _arg2); void cpu_idle(void); void cpu_switch(void); void cpu_throw(void) __dead2; -void unsleep(struct thread *); -void userret(struct thread *, struct trapframe *, u_int); +void unsleep(struct thread *_td); +void userret(struct thread *_td, struct trapframe *_tf, u_int _arg); -void cpu_exit(struct thread *); -void cpu_sched_exit(struct thread *); -void exit1(struct thread *, int) __dead2; -void cpu_fork(struct thread *, struct proc *, struct thread *, int); -void cpu_set_fork_handler(struct thread *, void (*)(void *), void *); -void cpu_wait(struct proc *); +void cpu_exit(struct thread *_td); +void
Re: Style fixups for proc.h
Julian Elischer writes: I don't know about the protection with a '_'. It's not standard and usually the name matches that used in the actual function. When the prototype parameter name matches a local variable, the C compiler (and lint) whine about clashes between names in local/global namespace. 2 ways to fix this are to protect the prototype argument names with the _, or to remove the argument name altogether. proc.h has no clear guidance, in that recent commits don't stick to the established style of the file. Some newish prototypes have a mixture of named/unnamed args in the same function. While I was making all prototypes' args named, I protected them. I'd like to fix the warnings, and I'd like the file to be consistent WRT argument naming. It's certainly not part of style(9) that I've ever noticed and it's generally noy done that way.. is there a move to do this on all the other files? There is a move to fix lint(1) warnings. M -- Mark Murray iumop ap!sdn w,I idlaH To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
On Sat, 1 Feb 2003, Mark Murray wrote: Julian Elischer writes: I don't know about the protection with a '_'. It's not standard and usually the name matches that used in the actual function. When the prototype parameter name matches a local variable, the C compiler (and lint) whine about clashes between names in local/global namespace. 2 ways to fix this are to protect the prototype argument names with the _, or to remove the argument name altogether. I'd rather have the names removed altogether. -- Dan Eischen To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
:Julian Elischer writes: : I don't know about the protection with a '_'. : : It's not standard and usually the name matches that used in the actual : function. : :When the prototype parameter name matches a local variable, the C compiler :(and lint) whine about clashes between names in local/global namespace. I've never in my life heard of this behavior before, what compiler arguments reproduce it? :2 ways to fix this are to protect the prototype argument names with the :_, or to remove the argument name altogether. If it is a problem, why not simply use the same variable names that are declared in the procedure proper? The underscore looks ugly and out of place and doesn't make that much sense to me. -Matt To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
:When the prototype parameter name matches a local variable, the C compiler :(and lint) whine about clashes between names in local/global namespace. I've never in my life heard of this behavior before, what compiler arguments reproduce it? WARNS=5. :2 ways to fix this are to protect the prototype argument names with the :_, or to remove the argument name altogether. If it is a problem, why not simply use the same variable names that are declared in the procedure proper? The underscore looks ugly and out of place and doesn't make that much sense to me. Because this doesn't always help, or if it did, the diffs are often much bigger and to many more files. M -- Mark Murray iumop ap!sdn w,I idlaH To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
On Sat, Feb 01, 2003 at 03:04:32PM -0800, Julian Elischer wrote: I don't know about the protection with a '_'. It's not standard and usually the name matches that used in the actual function. It's certainly not part of style(9) that I've ever noticed and it's generally noy done that way.. is there a move to do this on all the other files? man 9 style In header files visible to userland applications, prototypes that are visible must use either ``protected'' names (ones beginning with an underscore) or no names with the types. It is preferable to use pro- tected names. E.g., use: voidfunction(int); or: voidfunction(int _fd); -- Steve To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
Julian Elischer writes: I don't know about the protection with a '_'. It's not standard and usually the name matches that used in the actual function. When the prototype parameter name matches a local variable, the C compiler (and lint) whine about clashes between names in local/global namespace. According to C99, a function prototype has its own scope or name space. It terminates at the end of the function declarator. Basically naming a parameter in a function prototype is an aide to the human user; it is not needed for correct compilation[1] so this warning is bogus. As the spec says in section 6.7.5.3 (according the draft I have) The identifiers [naming parameters] are declared for descriptive purposes only and go out of scope at the end of the [prototype] declaration. I can't see what actual error is avoided by this warning. 2 ways to fix this are to protect the prototype argument names with the _, or to remove the argument name altogether. Why not fix the compiler lint instead of cluttering up declarations? -- bakul [1] Except for what is needed for declaring flexible or variable length array parameters. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
* De: Bakul Shah [EMAIL PROTECTED] [ Data: 2003-02-01 ] [ Subjecte: Re: Style fixups for proc.h ] Julian Elischer writes: I don't know about the protection with a '_'. It's not standard and usually the name matches that used in the actual function. When the prototype parameter name matches a local variable, the C compiler (and lint) whine about clashes between names in local/global namespace. According to C99, a function prototype has its own scope or name space. It terminates at the end of the function declarator. Basically naming a parameter in a function prototype is an aide to the human user; it is not needed for correct compilation[1] so this warning is bogus. As the spec says in section 6.7.5.3 (according the draft I have) The identifiers [naming parameters] are declared for descriptive purposes only and go out of scope at the end of the [prototype] declaration. I can't see what actual error is avoided by this warning. If a named prototype clashes with something in global scope, isn't it still a shadowing issue? They should probably never be *in* scope. -- Juli Mallett [EMAIL PROTECTED] AIM: BSDFlata -- IRC: juli on EFnet OpenDarwin, Mono, FreeBSD Developer ircd-hybrid Developer, EFnet addict FreeBSD on MIPS-Anything on FreeBSD To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
:WARNS=5. This isn't helpful. I tried adding every -W switch in bsd.sys.mk and couldn't reproduce the problem. What compiler option is causing the problem? : :2 ways to fix this are to protect the prototype argument names with the : :_, or to remove the argument name altogether. : : If it is a problem, why not simply use the same variable names that are : declared in the procedure proper? The underscore looks ugly and out of : place and doesn't make that much sense to me. : :Because this doesn't always help, or if it did, the diffs are often :much bigger and to many more files. : :M :-- :Mark Murray :iumop ap!sdn w,I idlaH Ok, now I'm really confused. How can it not always help? If the arguments are the same as the arguments declared in the underlying procedures why would an error still be produced? The diff you produced for proc.h is *already* fairly extensive. If you want to fix this, you only need to fix the lines generating compiler warnings. I really dislike screwing around with source code to work around bugs in the the compiler, or lint. Given the choice of underlines or leaving the arguments unnamed, I would leave them unnamed. Or I would figure out and remove whatever broken compiler option is generating the warning in the first place. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
Why not fix the compiler lint instead of cluttering up declarations? Can we at least get proc.h to have a consistent style of function parameter usage? M -- Mark Murray iumop ap!sdn w,I idlaH To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
:If a named prototype clashes with something in global scope,
:isn't it still a shadowing issue? They should probably never
:be *in* scope.
:--
:Juli Mallett [EMAIL PROTECTED]
:AIM: BSDFlata -- IRC: juli on EFnet
I finally was able to reproduce the bug. But it's an obvious bug in the
compiler at least in regards to named arguments in prototypes.
The -Wshadow switch causes the error message to be produced and it
occurs both for named arguments in prototypes and, named arguments
in the procedure proper, and named locals in the procedure.
The solution is simply to name the arguments in the prototype the same
as the arguments in the procedure. If we are going to use -Wshadow then
the arguments in the procedure will generate the error too so naming
the prototype arguments the same will not make things any better OR worse.
So the prototype arguments should either be named the same as those in the
procedure proper, or should not be named at all.
We definitely should not go adding underscores to the prototypes to
work around this problem.
-Matt
Matthew Dillon
[EMAIL PROTECTED]
int x;
int fubar(int x);
int
fubar(int y)
{
int x = 2;
++x;
y = 1;
return(1);
}
int
fubar2(int x)
{
++x;
return(1);
}
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
On Sat, 01 Feb 2003 16:02:57 -0800, Bakul Shah [EMAIL PROTECTED] said:
I can't see what actual error is avoided by this warning.
It's a potential error -- if there were an actual error, it would be
an error and not a warning.
The issue is simple:
Say you have an object and a function declared in global scope:
int foo;
/* many lines of declarations */
/* perhaps this is even in a different file */
void bar(quux_t foo);
At some point, somebody changes the spelling of `foo' and adds a
preprocessor macro for compatibility:
union baz {
int foo;
struct frotz *gorp;
} foobaz;
#define foo foobaz.foo
What happens to the declaration of that function? Well,
void bar(quux_t foobaz.foo);
is a syntax error.
My personal opinion, which is different from what style(9) recommends,
is that parameter names should be omitted for all functions, EXCEPT
those with ambiguous parameter types. Most functions in the kernel
only operate on one object of a given type at a time, so even without
the names it is (or should be) obvious what the object is used for.
Some functions, however, take multiple objects of the same type (e.g.,
two different sorts of flags); in these cases some additional help may
be warranted.
In the case of user headers, if parameter names are included, they
MUST be protected, because they would otherwise be polluting the
user's namespace. Hence, most headers historically do not use
parameter names.
-GAWollman
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
On Sat, 1 Feb 2003 19:31:47 -0500 (EST), I wrote:
union baz {
int foo;
struct frotz *gorp;
} foobaz;
#define foo foobaz.foo
Oops... What I meant to say:
union baz {
int bazu_foo;
struct frotz *bazu_gorp;
} foobaz;
#define foo foobaz.bazu_foo
With this emendation, my post will actually make sense.
-GAWollman
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
Matthew Dillon writes: :WARNS=5. This isn't helpful. I tried adding every -W switch in bsd.sys.mk and couldn't reproduce the problem. What compiler option is causing the problem? I don't know which specific one. Ok, now I'm really confused. How can it not always help? If the arguments are the same as the arguments declared in the underlying procedures why would an error still be produced? The diff you produced for proc.h is *already* fairly extensive. If you want to fix this, you only need to fix the lines generating compiler warnings. arg in a function prototype gets confused with variable arg in some function(s). I really dislike screwing around with source code to work around bugs in the the compiler, or lint. Given the choice of underlines or leaving the arguments unnamed, I would leave them unnamed. Or I would figure out and remove whatever broken compiler option is generating the warning in the first place. Then can we just get the proc.h prototypes into a (any) consistent style? M -- Mark Murray iumop ap!sdn w,I idlaH To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
:I really dislike screwing around with source code to work around :bugs in the the compiler, or lint. Given the choice of underlines :or leaving the arguments unnamed, I would leave them unnamed. Or I :would figure out and remove whatever broken compiler option is generating :the warning in the first place. : :Then can we just get the proc.h prototypes into a (any) consistent :style? : :M :-- :Mark Murray Lets ask ourselves what the goal of the named prototypes is... the compiler doesn't need them, obviously, so one would presume that the goal is human readability. So if we care about human readability we should simply name them after the argument names used in the procedures proper. If we don't care about human readability we should omit the names entirely. An underscore would be detrimental to human readability. It makes the prototypes look rather nasty when I look at the fully patched proc.h, and also makes them different from the arguments as declared in the procedures proper. A quick perusal of include files shows that we use a mix. Examples: sys/acl.h -- looks like the authors tried to use the underscore technique but forgot a couple. sys/aio.h -- a mix of named (without underscore) and unnamed. sys/blist.h -- named prototypes without underscore (mine originally) sys/buf.h -- a mix of named (without underscore) and unnamed. Mostly unnamed, and __P() is still being used. (the named one is probably mine). sys/callout.h -- unnamed. sys/conf.h -- mostly named (without underscore) (not mine) sys/cons.h -- unnamed And it goes on. Quite a mess we have, actually. We still have __P in many places. The newest header file would arguably be acl.h in which the author used underscores. I can't say I like the way it reads on the screen. Older header files either still have __P, don't have __P and the arguments are named (typically without an underscore), or mix with some of the arguments named and some not (some wholely not). -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
I can't see what actual error is avoided by this warning.
s/actual/potential/
If a named prototype clashes with something in global scope,
isn't it still a shadowing issue? They should probably never
be *in* scope.
Nothing is being shadowed. Paramater names in a function
prototype (as opposed to a function definition) are simply
not relevant to the compiler (their distinctness, type and
positions are but not the actual names). No potential bug is
being hidden by saying, for example,
int x;
int foo(int x);
It is perfectly fine to later define foo as
int foo(int y)
{
return x + y;
}
The compiler should simply discard any parameter names in a
prototype once the prototype is digested and C programmers
need to know that. Now if parameter y shadows a global y one
may want to be warned about it.
Garrett gives an example:
Say you have an object and a function declared in global scope:
int foo;
/* many lines of declarations */
/* perhaps this is even in a different file */
void bar(quux_t foo);
At some point, somebody changes the spelling of `foo' and adds a
preprocessor macro for compatibility:
union baz {
int foo;
struct frotz *gorp;
} foobaz;
#define foo foobaz.foo
What happens to the declaration of that function? Well,
void bar(quux_t foobaz.foo);
is a syntax error.
This sort of problem can occur when you have any two objects named the
same. Consider:
struct dumb { int foo; };
After the above redefinition this defn won't compile (even
after his amendation:-).
Not naming parameters is one solution but with some loss of
perspicuity. Consider:
int* make_matrix(int, int);
versus
int* make_matrix(int rows, int columns);
-- bakul
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
On Sun, 2 Feb 2003, Mark Murray wrote: Why not fix the compiler lint instead of cluttering up declarations? Can we at least get proc.h to have a consistent style of function parameter usage? Sure. let's be consistent with all the other .h files in the kernel. what does a quick census show? M -- Mark Murray iumop ap!sdn w,I idlaH To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
On 2003-02-01 19:31, Garrett Wollman [EMAIL PROTECTED] wrote: On Sat, 01 Feb 2003 16:02:57 -0800, Bakul Shah [EMAIL PROTECTED] said: I can't see what actual error is avoided by this warning. My personal opinion, which is different from what style(9) recommends, is that parameter names should be omitted for all functions, EXCEPT those with ambiguous parameter types. This is what I am almost inclined to agree with too. But then, headers are one of the sources of information for newbie programmers too. I have learned a lot of stuff by reading headers and not having names in any prototype would arguably make it harder to use the prototypes of functions in headers to learn things :-/ To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
Matthew Dillon wrote:
Mark Murray wrote:
:Then can we just get the proc.h prototypes into a (any) consistent
:style?
Yes; no brainer, except where Garrett has indicated (e.g. a
function that takes a fromvp and tovp, or other similar arguments).
Lets ask ourselves what the goal of the named prototypes is... the
compiler doesn't need them, obviously, so one would presume that the
goal is human readability.
Technically, the compiler doesn't need prototypes at all; they
are merely a band-aid for the compiler vendors, who did not want
to have to deal with changing object file format. Because of
that, we also have symbol decoration for C++, and we have a C++
compiler that requires explicit declaration of some functions
which should be implicit. If the object file had attribution,
and you wanted to make implied coercion, the linker could emit
glue, as necessary, and be done with it: no prototypes in scope.
If you wanted to disallow implied coercion, it could be a link-time
error. Either way, the same complaining could take place, with
no need to abuse the compiler semantics.
Prototypes, in general, are things for compiler vendors, and the
goal of the named arguments in a prototype was, at least at one
point, to help out ANSI C compilers that wanted to requse the
function declaration parsing code, and would barf without some
token there to jam into a symbol table. The shadowing warning
that is being complained about by Mark Murray is, as has been
pointed out, a compiler bug: according to the standards, the
symbols are supposed to go out of scope anyway. In fact, it should
ignore tokens to the coma or closing parenthesis, and not be bitching
in the first place.
So if we care about human readability we should simply name them after
the argument names used in the procedures proper. If we don't care
about human readability we should omit the names entirely.
Yes, this makes sense. The grip that Garrett brought up is an
artifact of the compiler being stupid. In fact, the same problem
would occur for the function declaration, and all compilation units
for which there are header files containing prototypes should include
those headers themselves, in order to ensure the prototype was in
scope at the time the function was encountered, to insure against
trivial errors from the prototype not matching the function declaration.
Thus, if the prototype is:
int xxx(int _foo);
Then the function should be:
int xxx(int _foo)
{
}
...since what a
#define foo foobaz.bazu_foo
will do to a prototype declaration, it will surely do to the function
declaration as well, if it is in a header which is in scope at the
time the compiler encounters the actual function declaration.
An underscore would be detrimental to human readability. It makes the
prototypes look rather nasty when I look at the fully patched proc.h,
and also makes them different from the arguments as declared in the
procedures proper.
It's just ugly, in general, and confusing, when combined with
structure pointer element references. 8-(.
[ ... ]
And it goes on. Quite a mess we have, actually. We still have __P in
many places. The newest header file would arguably be acl.h in which
the author used underscores. I can't say I like the way it reads on the
screen. Older header files either still have __P, don't have __P and
the arguments are named (typically without an underscore), or mix with
some of the arguments named and some not (some wholely not).
So let Mark correct them into uniformity; that can be done
without protecting everything, which can be done as a seperate
protection or argument name removal pass.
-- Terry
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
On Sat 01 Feb, Steve Kargl wrote: From: Steve Kargl [EMAIL PROTECTED] To: Julian Elischer [EMAIL PROTECTED] Cc: Mark Murray [EMAIL PROTECTED], [EMAIL PROTECTED], [EMAIL PROTECTED] Subject: Re: Style fixups for proc.h On Sat, Feb 01, 2003 at 03:04:32PM -0800, Julian Elischer wrote: I don't know about the protection with a '_'. It's not standard and usually the name matches that used in the actual function. It's certainly not part of style(9) that I've ever noticed and it's generally noy done that way.. is there a move to do this on all the other files? man 9 style In header files visible to userland applications, prototypes that are visible must use either ``protected'' names (ones beginning with an underscore) or no names with the types. It is preferable to use pro- tected names. E.g., use: voidfunction(int); or: voidfunction(int _fd); Since having actual names in can be helpful if the names are relevant, but having dozens of *_p floating all over the place is not more easily readable, why not leave names out completely when they are not relevant and protect with the underscore when they are? This agrees with style(9). Andrew To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h
Well, there is something to be said for trying to avoid userland namespace pollution, but it is still somewhat of a stretch since most userland programs #include standard and system headers before they #include their own, and the includes are typically done before any code. But I see no reason why the underscore methodology would need to be used for kernelland prototypes. C has its problems and we need to live with them, but we shouldn't have to add bogus underscores to prototyped arguments to work around those problems. I'd prefer normally named arguments but if I were given only a choice between underscored named arguments and unnamed arguments, I'd take unnamed arguments hands down. -Matt :Actually, the pattern is that the function prototypes exposed to userspace :are prefixed with '_' to prevent interfering with the application :namespace. The ones exposed only in the kernel don't. I should probably :update the kernel ones as well. This is mostly because of the profound :evils associated with the C preprocessor, which can cause substitutions to :occur in function prototypes (this is often used intentionally). : :For an example of this evil: there appears to be a convention in which we :name structure elements with a prefix, such as m_blah, based on the :structure name. At one point I added m_flags to struct mac. When I :included mbuf.h, this became m_hdr.mh_flags, resulting in fairly obtuse :compile errors. Protecting user applications against hard to understand :compile errors is an important part of providing useful include files to :application writers, so avoiding exposing things to the application :namespace where it can be avoided. : :Robert N M Watson FreeBSD Core Team, TrustedBSD Projects To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: Style fixups for proc.h by andrew@driftin.net
On Sat 01 Feb, Matthew Dillon wrote: Well, there is something to be said for trying to avoid userland namespace pollution, but it is still somewhat of a stretch since most userland programs #include standard and system headers before they #include their own, and the includes are typically done before any code. But I see no reason why the underscore methodology would need to be used for kernelland prototypes. C has its problems and we need to live with them, but we shouldn't have to add bogus underscores to prototyped arguments to work around those problems. I'd prefer normally named arguments but if I were given only a choice between underscored named arguments and unnamed arguments, I'd take unnamed arguments hands down. As has been said earlier in this thread, having named arguments can often help new coders learn and help readability (one knows what an argument is for from looking at the header file as opposed to having to look through the C file), which is why I suggested having underscored named arguments when they are useful to have named, and no names when naming them is not useful. Andrew To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
