Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-22 Thread Wojciech A. Koszek
On Mon, Dec 19, 2005 at 02:24:04PM -0500, John Baldwin wrote:
 On Friday 16 December 2005 05:19 pm, Wojciech A. Koszek wrote:
  On Fri, Dec 16, 2005 at 11:14:12AM -0500, John Baldwin wrote:
   On Friday 16 December 2005 04:10 am, Peter Jeremy wrote:
On Thu, 2005-Dec-15 22:37:45 +, Wojciech A. Koszek wrote:
   abort2(const char *why, int nargs, void **args);

why is reason of program abort, nargs is number of arguments
passed in args. Both why and args (with %p format) will be
printed via log(9). Sample output:
[..]
pid 3004 abort2 abort2: ABORT2 arg0:0x62612f2e
pid 3019 abort2 abort2: invalid argument
[..]
   
I don't believe the following code is correct.  uap-args is a
userspace pointer so uap-args[i] is dereferencing a userspace
argument in kernelspace.
+   arg = uargs[i] = (void *) fuword(uap-args[i]);
I think it should be fuword(uap-args + i);
   
I don't see the point of the following test.  arg is printed using
%p and never de-referenced so there's no reason it can't be NULL.  I
would see that a legitimate use of abort2() is when the application
detects that a pointer is unexpectedly NULL.  Aborting on -1 is less
clear - if fuword() fails, it will return -1 but, equally, a faulty
user application may have left -1 in a pointer.  (Note that mmap(2)
returns -1 on error so it's not inconceivable that a pointer could
contain -1).
   
+   /* Prevent from faults in user-space */
+   if (arg == NULL || arg == (void *)-1) {
+   error = EINVAL;
+   break;
+   }
   
Taking the above into account, I believe the code should be:
+   if (uap-args == NULL)
+   break;
+   error = copyin(uap-args, uargs, uap-nargs * sizeof
(void *)); +   if (error != 0)
+   break;
  
   Agreed.  Also, copyinstr() can provide a better interface for copying the
   why string in.  Also, the PROC LOCK isn't needed for reading the static
   p_pid and p_comm fields of struct proc.  Also, I second the other
   comments of do { } while(0) vs goto.  Many existing syscalls use 'goto
   out;' for error handling, and I think that is one of the very few cases
   when goto is useful and not harmful.
 
  Thanks for the suggestions and comments!
 
  My question is related with copying string from user-space: the only
  difference I can see between those functions (other than operating of
  strings/sbufs) is that sbuf_copyin() looses 'done' [1]. Since current
  abort2() makes use of sbuf(9), I'll have to have additional buffer just
  for string copying and than copy it to sbuf. Would you prefer this
  solution or complete migration from sbufs to strl..()?
 
  [1] Couldn't sbuf_copyin() simply return 'done' from copyinstr()
  embedded in it, since it already returns -1 on failure? This function
  is used in two places, which make no use of return value.
 
 That sounds good to me (fixing sbuf_copyin()).
 

I got no response from people for whom changing this function could be a
problem.

My patch is here:
http://freebsd.czest.pl/dunstan/FreeBSD/sbuf_copyin.0.patch

-- 
* Wojciech A. Koszek  [EMAIL PROTECTED]
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-22 Thread Wojciech A. Koszek
On Fri, Dec 16, 2005 at 11:14:12AM -0500, John Baldwin wrote:
 On Friday 16 December 2005 04:10 am, Peter Jeremy wrote:
  On Thu, 2005-Dec-15 22:37:45 +, Wojciech A. Koszek wrote:
 abort2(const char *why, int nargs, void **args);
  
  why is reason of program abort, nargs is number of arguments
  passed in args. Both why and args (with %p format) will be
  printed via log(9). Sample output:
  [..]
  pid 3004 abort2 abort2: ABORT2 arg0:0x62612f2e
  pid 3019 abort2 abort2: invalid argument
  [..]
[..]
 
 Agreed.  Also, copyinstr() can provide a better interface for copying the why 
 string in.  Also, the PROC LOCK isn't needed for reading the static p_pid and 
 p_comm fields of struct proc.  Also, I second the other comments of do { } 
 while(0) vs goto.  Many existing syscalls use 'goto out;' for error handling, 
 and I think that is one of the very few cases when goto is useful and not 
 harmful.

Updated patch is here:

http://freebsd.czest.pl/dunstan/FreeBSD/abort2/abort2.3.patch

If I have to change something, let me know. Once again -- comments are
welcome.

-- 
* Wojciech A. Koszek  [EMAIL PROTECTED]
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-19 Thread John Baldwin
On Friday 16 December 2005 05:19 pm, Wojciech A. Koszek wrote:
 On Fri, Dec 16, 2005 at 11:14:12AM -0500, John Baldwin wrote:
  On Friday 16 December 2005 04:10 am, Peter Jeremy wrote:
   On Thu, 2005-Dec-15 22:37:45 +, Wojciech A. Koszek wrote:
abort2(const char *why, int nargs, void **args);
   
   why is reason of program abort, nargs is number of arguments
   passed in args. Both why and args (with %p format) will be
   printed via log(9). Sample output:
   [..]
   pid 3004 abort2 abort2: ABORT2 arg0:0x62612f2e
   pid 3019 abort2 abort2: invalid argument
   [..]
  
   I don't believe the following code is correct.  uap-args is a
   userspace pointer so uap-args[i] is dereferencing a userspace
   argument in kernelspace.
   +   arg = uargs[i] = (void *) fuword(uap-args[i]);
   I think it should be fuword(uap-args + i);
  
   I don't see the point of the following test.  arg is printed using
   %p and never de-referenced so there's no reason it can't be NULL.  I
   would see that a legitimate use of abort2() is when the application
   detects that a pointer is unexpectedly NULL.  Aborting on -1 is less
   clear - if fuword() fails, it will return -1 but, equally, a faulty
   user application may have left -1 in a pointer.  (Note that mmap(2)
   returns -1 on error so it's not inconceivable that a pointer could
   contain -1).
  
   +   /* Prevent from faults in user-space */
   +   if (arg == NULL || arg == (void *)-1) {
   +   error = EINVAL;
   +   break;
   +   }
  
   Taking the above into account, I believe the code should be:
   +   if (uap-args == NULL)
   +   break;
   +   error = copyin(uap-args, uargs, uap-nargs * sizeof
   (void *)); +   if (error != 0)
   +   break;
 
  Agreed.  Also, copyinstr() can provide a better interface for copying the
  why string in.  Also, the PROC LOCK isn't needed for reading the static
  p_pid and p_comm fields of struct proc.  Also, I second the other
  comments of do { } while(0) vs goto.  Many existing syscalls use 'goto
  out;' for error handling, and I think that is one of the very few cases
  when goto is useful and not harmful.

 Thanks for the suggestions and comments!

 My question is related with copying string from user-space: the only
 difference I can see between those functions (other than operating of
 strings/sbufs) is that sbuf_copyin() looses 'done' [1]. Since current
 abort2() makes use of sbuf(9), I'll have to have additional buffer just
 for string copying and than copy it to sbuf. Would you prefer this
 solution or complete migration from sbufs to strl..()?

 [1] Couldn't sbuf_copyin() simply return 'done' from copyinstr()
 embedded in it, since it already returns -1 on failure? This function
 is used in two places, which make no use of return value.

That sounds good to me (fixing sbuf_copyin()).

-- 
John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/
Power Users Use the Power to Serve  =  http://www.FreeBSD.org
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-16 Thread Peter Jeremy
On Fri, 2005-Dec-16 00:17:14 +0100, Václav Haisman wrote:
I would like to comment on FreeBSD style(9) a bit.

That is a different bikeshed.  If, taking into account the other
comments in this thread, you believe that you can justify changes to
style(9), please start a new thread - probably in -arch.

-- 
Peter Jeremy
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-16 Thread Peter Jeremy
On Thu, 2005-Dec-15 22:37:45 +, Wojciech A. Koszek wrote:
   abort2(const char *why, int nargs, void **args);

why is reason of program abort, nargs is number of arguments
passed in args. Both why and args (with %p format) will be
printed via log(9). Sample output:
[..]
pid 3004 abort2 abort2: ABORT2 arg0:0x62612f2e
pid 3019 abort2 abort2: invalid argument
[..]

I don't believe the following code is correct.  uap-args is a
userspace pointer so uap-args[i] is dereferencing a userspace
argument in kernelspace.
+   arg = uargs[i] = (void *) fuword(uap-args[i]);
I think it should be fuword(uap-args + i);

I don't see the point of the following test.  arg is printed using
%p and never de-referenced so there's no reason it can't be NULL.  I
would see that a legitimate use of abort2() is when the application
detects that a pointer is unexpectedly NULL.  Aborting on -1 is less
clear - if fuword() fails, it will return -1 but, equally, a faulty
user application may have left -1 in a pointer.  (Note that mmap(2)
returns -1 on error so it's not inconceivable that a pointer could
contain -1).

+   /* Prevent from faults in user-space */
+   if (arg == NULL || arg == (void *)-1) {
+   error = EINVAL;
+   break;
+   }

Taking the above into account, I believe the code should be:
+   if (uap-args == NULL)
+   break;
+   error = copyin(uap-args, uargs, uap-nargs * sizeof (void *));
+   if (error != 0)
+   break;

-- 
Peter Jeremy
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-16 Thread John Baldwin
On Friday 16 December 2005 04:10 am, Peter Jeremy wrote:
 On Thu, 2005-Dec-15 22:37:45 +, Wojciech A. Koszek wrote:
  abort2(const char *why, int nargs, void **args);
 
 why is reason of program abort, nargs is number of arguments
 passed in args. Both why and args (with %p format) will be
 printed via log(9). Sample output:
 [..]
 pid 3004 abort2 abort2: ABORT2 arg0:0x62612f2e
 pid 3019 abort2 abort2: invalid argument
 [..]

 I don't believe the following code is correct.  uap-args is a
 userspace pointer so uap-args[i] is dereferencing a userspace
 argument in kernelspace.
 +   arg = uargs[i] = (void *) fuword(uap-args[i]);
 I think it should be fuword(uap-args + i);

 I don't see the point of the following test.  arg is printed using
 %p and never de-referenced so there's no reason it can't be NULL.  I
 would see that a legitimate use of abort2() is when the application
 detects that a pointer is unexpectedly NULL.  Aborting on -1 is less
 clear - if fuword() fails, it will return -1 but, equally, a faulty
 user application may have left -1 in a pointer.  (Note that mmap(2)
 returns -1 on error so it's not inconceivable that a pointer could
 contain -1).

 +   /* Prevent from faults in user-space */
 +   if (arg == NULL || arg == (void *)-1) {
 +   error = EINVAL;
 +   break;
 +   }

 Taking the above into account, I believe the code should be:
 +   if (uap-args == NULL)
 +   break;
 +   error = copyin(uap-args, uargs, uap-nargs * sizeof (void
 *)); +   if (error != 0)
 +   break;

Agreed.  Also, copyinstr() can provide a better interface for copying the why 
string in.  Also, the PROC LOCK isn't needed for reading the static p_pid and 
p_comm fields of struct proc.  Also, I second the other comments of do { } 
while(0) vs goto.  Many existing syscalls use 'goto out;' for error handling, 
and I think that is one of the very few cases when goto is useful and not 
harmful.

-- 
John Baldwin [EMAIL PROTECTED]http://www.FreeBSD.org/~jhb/
Power Users Use the Power to Serve  =  http://www.FreeBSD.org
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-16 Thread Wojciech A. Koszek
On Fri, Dec 16, 2005 at 08:10:57PM +1100, Peter Jeremy wrote:
 On Thu, 2005-Dec-15 22:37:45 +, Wojciech A. Koszek wrote:
  abort2(const char *why, int nargs, void **args);
 
 why is reason of program abort, nargs is number of arguments
 passed in args. Both why and args (with %p format) will be
 printed via log(9). Sample output:
 [..]
 pid 3004 abort2 abort2: ABORT2 arg0:0x62612f2e
 pid 3019 abort2 abort2: invalid argument
 [..]
 
 I don't believe the following code is correct.  uap-args is a
 userspace pointer so uap-args[i] is dereferencing a userspace
 argument in kernelspace.
 +   arg = uargs[i] = (void *) fuword(uap-args[i]);
 I think it should be fuword(uap-args + i);
 
 I don't see the point of the following test.  arg is printed using
 %p and never de-referenced so there's no reason it can't be NULL.  I
 would see that a legitimate use of abort2() is when the application
 detects that a pointer is unexpectedly NULL.  Aborting on -1 is less
 clear - if fuword() fails, it will return -1 but, equally, a faulty
 user application may have left -1 in a pointer.  (Note that mmap(2)
 returns -1 on error so it's not inconceivable that a pointer could
 contain -1).
 
 +   /* Prevent from faults in user-space */
 +   if (arg == NULL || arg == (void *)-1) {
 +   error = EINVAL;
 +   break;
 +   }
 
 Taking the above into account, I believe the code should be:
 +   if (uap-args == NULL)
 +   break;
 +   error = copyin(uap-args, uargs, uap-nargs * sizeof (void 
 *));
 +   if (error != 0)
 +   break;

Of course! Thanks for this comment! Updated version should be available
soon.

-- 
* Wojciech A. Koszek  [EMAIL PROTECTED]
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-16 Thread Wojciech A. Koszek
On Fri, Dec 16, 2005 at 11:14:12AM -0500, John Baldwin wrote:
 On Friday 16 December 2005 04:10 am, Peter Jeremy wrote:
  On Thu, 2005-Dec-15 22:37:45 +, Wojciech A. Koszek wrote:
 abort2(const char *why, int nargs, void **args);
  
  why is reason of program abort, nargs is number of arguments
  passed in args. Both why and args (with %p format) will be
  printed via log(9). Sample output:
  [..]
  pid 3004 abort2 abort2: ABORT2 arg0:0x62612f2e
  pid 3019 abort2 abort2: invalid argument
  [..]
 
  I don't believe the following code is correct.  uap-args is a
  userspace pointer so uap-args[i] is dereferencing a userspace
  argument in kernelspace.
  +   arg = uargs[i] = (void *) fuword(uap-args[i]);
  I think it should be fuword(uap-args + i);
 
  I don't see the point of the following test.  arg is printed using
  %p and never de-referenced so there's no reason it can't be NULL.  I
  would see that a legitimate use of abort2() is when the application
  detects that a pointer is unexpectedly NULL.  Aborting on -1 is less
  clear - if fuword() fails, it will return -1 but, equally, a faulty
  user application may have left -1 in a pointer.  (Note that mmap(2)
  returns -1 on error so it's not inconceivable that a pointer could
  contain -1).
 
  +   /* Prevent from faults in user-space */
  +   if (arg == NULL || arg == (void *)-1) {
  +   error = EINVAL;
  +   break;
  +   }
 
  Taking the above into account, I believe the code should be:
  +   if (uap-args == NULL)
  +   break;
  +   error = copyin(uap-args, uargs, uap-nargs * sizeof (void
  *)); +   if (error != 0)
  +   break;
 
 Agreed.  Also, copyinstr() can provide a better interface for copying the why 
 string in.  Also, the PROC LOCK isn't needed for reading the static p_pid and 
 p_comm fields of struct proc.  Also, I second the other comments of do { } 
 while(0) vs goto.  Many existing syscalls use 'goto out;' for error handling, 
 and I think that is one of the very few cases when goto is useful and not 
 harmful.

Thanks for the suggestions and comments!

My question is related with copying string from user-space: the only
difference I can see between those functions (other than operating of
strings/sbufs) is that sbuf_copyin() looses 'done' [1]. Since current
abort2() makes use of sbuf(9), I'll have to have additional buffer just
for string copying and than copy it to sbuf. Would you prefer this
solution or complete migration from sbufs to strl..()?

[1] Couldn't sbuf_copyin() simply return 'done' from copyinstr()
embedded in it, since it already returns -1 on failure? This function
is used in two places, which make no use of return value.
-- 
* Wojciech A. Koszek  [EMAIL PROTECTED]
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


[CALL FOR TESTERS] New system call: abort2()

2005-12-15 Thread Wojciech A. Koszek
(discussed task was picked from Poul-Henning Kamp's TODO list)

Hackers,
I've implemented abort2() system call. Works just like abort(3), but
delivers signal reliably. Here is a prototype:

abort2(const char *why, int nargs, void **args);

why is reason of program abort, nargs is number of arguments
passed in args. Both why and args (with %p format) will be
printed via log(9). Sample output:

[..]
pid 3004 abort2 abort2: ABORT2 arg0:0x62612f2e
pid 3019 abort2 abort2: invalid argument
[..]

I put two versions: one implemented as KLD so that more people can test
it without a problem:

http://freebsd.czest.pl/dunstan/FreeBSD/abort2/abort2-0.3.tgz

..and also as a patch:

http://freebsd.czest.pl/dunstan/FreeBSD/abort2/diff.1.abort2-sycall

If I miss some tests, please let me know, especially in
user-kernelspace interaction area. (currently I run patched kernel and
couldn't get a panic in testing process).

Short description - function:

abort2(const char *why, int nargs, void **args);

..never returns. If improper arguments are used, instead of SIGABRT,
SIGKILL is delivered. By improper arguments I understand: invalid
pointers -- why or args or  0  nargs  16.

Comments are welcome!
-- 
* Wojciech A. Koszek  [EMAIL PROTECTED]
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-15 Thread Václav Haisman
Wojciech A. Koszek wrote:
[...]
 Comments are welcome!

As for the patch, the use of do {} while(0) instead of goto looks odd to me.

I would like to comment on FreeBSD style(9) a bit. Why does not mention
or even encourage C99 style // comments? They are nice when one wants to
comment out bigger chunks of code with /**/ comment.

On the similar note, the ability to move declarations closer to the
point of use in code is IMO nice feature, too. The style(9) doesn't
mention this either.


Vaclav Haisman


signature.asc
Description: OpenPGP digital signature


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-15 Thread joerg
On Fri, Dec 16, 2005 at 12:17:14AM +0100, Václav Haisman wrote:
 
 I would like to comment on FreeBSD style(9) a bit. Why does not mention
 or even encourage C99 style // comments? They are nice when one wants to
 comment out bigger chunks of code with /**/ comment.

Use #if 0 ... #endif for that.

Joerg

 On the similar note, the ability to move declarations closer to the
 point of use in code is IMO nice feature, too. The style(9) doesn't
 mention this either.

Because it is not necessarily a good thing.

Joerg
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-15 Thread Warner Losh
 I would like to comment on FreeBSD style(9) a bit. Why does not mention
 or even encourage C99 style // comments? They are nice when one wants to
 comment out bigger chunks of code with /**/ comment.

Too new.  /**/ comment out is bogus anyway.  #if 0 ... #endif is better.

 On the similar note, the ability to move declarations closer to the
 point of use in code is IMO nice feature, too. The style(9) doesn't
 mention this either.

C doesn't allow it, or didn't until recently.  That style tends to
lead to really gross things too.  Functions should be short enough
that it doesn't matter.

But changing style(9) is hard.  We have better things to do with our
lives. :-)

Warner
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-15 Thread Poul-Henning Kamp
In message [EMAIL PROTECTED], Warner Losh writes:

 On the similar note, the ability to move declarations closer to the
 point of use in code is IMO nice feature, too. The style(9) doesn't
 mention this either.

C doesn't allow it, or didn't until recently.  That style tends to
lead to really gross things too.  Functions should be short enough
that it doesn't matter.

Also, it tends to make it harder to judge the amount of stackspace
a function uses, something which is not entirely uninteresting in
kernel programming.

And yes, changing style(9) is just not worth the time it takes.

-- 
Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
[EMAIL PROTECTED] | TCP/IP since RFC 956
FreeBSD committer   | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-15 Thread Václav Haisman
[EMAIL PROTECTED] wrote:
 On Fri, Dec 16, 2005 at 12:17:14AM +0100, Václav Haisman wrote:
 
I would like to comment on FreeBSD style(9) a bit. Why does not mention
or even encourage C99 style // comments? They are nice when one wants to
comment out bigger chunks of code with /**/ comment.
 
 
 Use #if 0 ... #endif for that.
Well, that was the only option in time of C90, now we have IMO better
facility for this. Also most editors do not mark such code as commented
out which might a bit inconvenient.

 
 Joerg
 
 
On the similar note, the ability to move declarations closer to the
point of use in code is IMO nice feature, too. The style(9) doesn't
mention this either.
 
 
 Because it is not necessarily a good thing.
I am of different opinion but I will leave it at that.

 
 Joerg

Vaclav Haisman



signature.asc
Description: OpenPGP digital signature


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-15 Thread Václav Haisman


Poul-Henning Kamp wrote:
 In message [EMAIL PROTECTED], Warner Losh writes:
 
 
On the similar note, the ability to move declarations closer to the
point of use in code is IMO nice feature, too. The style(9) doesn't
mention this either.

C doesn't allow it, or didn't until recently.  That style tends to
lead to really gross things too.  Functions should be short enough
that it doesn't matter.
(I haven't gotten this email yet.)
Anything can lead to really gross things if misused.

 
 
 Also, it tends to make it harder to judge the amount of stackspace
 a function uses, something which is not entirely uninteresting in
 kernel programming.
While it might be harder to get estimate of stack space allocation I
suspect it could actually lower the allocation.

 
 And yes, changing style(9) is just not worth the time it takes.
 

Vaclav Haisman


signature.asc
Description: OpenPGP digital signature


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-15 Thread Warner Losh
  Also, it tends to make it harder to judge the amount of stackspace
  a function uses, something which is not entirely uninteresting in
  kernel programming.
 While it might be harder to get estimate of stack space allocation I
 suspect it could actually lower the allocation.

Maybe, maybe not.  Experience has shown that putting things in inner
scopes only sometimes reduces stack usage, and then it depends on the
compiler, options used, etc.

Warner
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: [CALL FOR TESTERS] New system call: abort2()

2005-12-15 Thread Wojciech A. Koszek
On Fri, Dec 16, 2005 at 12:17:14AM +0100, Václav Haisman wrote:
 Wojciech A. Koszek wrote:
 [...]
  Comments are welcome!
 
 As for the patch, the use of do {} while(0) instead of goto looks odd to me.

This can be changed easily in final version of the patch if needed.

 I would like to comment on FreeBSD style(9) a bit. Why does not mention
 or even encourage C99 style // comments? They are nice when one wants to
 comment out bigger chunks of code with /**/ comment.
 
 On the similar note, the ability to move declarations closer to the
 point of use in code is IMO nice feature, too. The style(9) doesn't
 mention this either.

This creates unnecessary problems: large blocks can duplicate name of
the variable so that they overlap, which has happened in the past.
Additionally, some files use this kind of declaration, and it makes
source hard to read.

Also please note that current comments probably needs changing. I'm
looking forward to hearing new comments in that field.

Thanks,
-- 
* Wojciech A. Koszek  [EMAIL PROTECTED]
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]