Date: Tue, 19 May 2015 10:58:31 -0500 From: Pedro Giffuni <pfg@FreeBSD.org> To: John Baldwin <jhb@freebsd.org> Cc: Bruce Evans <brde@optusnet.com.au>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r283088 - head/sys/ddb Message-ID: <555B5DA7.5000207@FreeBSD.org> In-Reply-To: <2053555.dpIzi23R03@ralph.baldwin.cx> References: <201505182227.t4IMRljx078812@svn.freebsd.org> <20150519135341.R2157@besplex.bde.org> <BA474AEC-A0A8-4FF8-8881-397E8280C72F@FreeBSD.org> <2053555.dpIzi23R03@ralph.baldwin.cx>
next in thread | previous in thread | raw e-mail | index | archive | help
On 05/19/15 08:45, John Baldwin wrote: > On Tuesday, May 19, 2015 12:28:05 AM Pedro Giffuni wrote: >>>>>> Modified: head/sys/ddb/db_break.c >>>>>> ============================================================================== >>>>>> --- head/sys/ddb/db_break.c Mon May 18 22:14:06 2015 (r283087) >>>>>> +++ head/sys/ddb/db_break.c Mon May 18 22:27:46 2015 (r283088) >>>>>> @@ -155,12 +155,12 @@ db_find_breakpoint_here(db_addr_t addr) >>>>>> return db_find_breakpoint(db_map_addr(addr), addr); >>>>>> } >>>>>> >>>>>> -static boolean_t db_breakpoints_inserted = TRUE; >>>>>> +static boolean_t db_breakpoints_inserted = true; >>>>> This code hasn't been churned to use the boolean type. It still uses >>>>> boolean_t, which is plain int. TRUE and FALSE go with this type. true >>>>> and false go with the boolean type. This probably makes no difference, >>>>> because TRUE happens to be implemented with the same value as true and >>>>> there are lots of implicit versions between the types. >>>> Yes, I noticed the return types are still ints. It doesn’t look difficult >>>> to convert it to use a real boolean type. In any case, I would prefer to go >>>> forward (using bool) instead of reverting this change. >>> That wuld be sideways. >>> >>> I forgot to mention (again) in my previous reply that boolean_t is a mistake >>> by me. KNF code doesn't even use the ! operator, but uses explicit >>> comparison with 0. The boolean_t type and TRUE and FALSE are from Mach. >>> They were used mainly in ddb and vm, and are still almost never used in >>> kern. I used to like typedefs and a typedef for boolean types, and didn't >>> know KNF very well, so in 1995 I moved the declaration of boolean_t from >>> Mach vm code to sys/types.h to try to popularize it. This was a mistake. >>> Fortunately, it is still rarely used in core kernel code. >>> >>> The boolean type is also almost never used for syscalls. In POSIX.1-2001, >>> <stdbool.h> is inherited from C99, but is never used for any other POSIX >>> API. Using it for syscalls would mainly cause portability problems. >>> >> OK, I do understand the kernel wants to keep the C dialect somewhat limited, >> and adding stdbool.h doesn’t buy us any type safety here. >> >> I’ll revert the change (prob. tomorrow though). > I will disagree with Bruce a bit and put my vote in for replacing boolean_t > with bool where it is used. I do think that logically (if not strictly) your > commit is a type mismatch as TRUE/FALSE is for boolean_t and true/false are > for bool. I agree with Bruce that we probably don't want to use bool for > system calls. However, I think using bool in the kernel itself is ok and that > we should replace boolean_t with bool. > I guess it boils down to the dilemma between modernity and common practice. OK, I know the current change can't stay as-is, and even Bruce admits that boolean_t is a mistake, so I think I will give the bool a try. Thanks for the feedback! Pedro.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?555B5DA7.5000207>