un-boolean_t ANSIfy ddb(4) for hppa & sparc64

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

un-boolean_t ANSIfy ddb(4) for hppa & sparc64

Martin Pieuchot
ok?

Index: arch/hppa/hppa/db_disasm.c
===================================================================
RCS file: /cvs/src/sys/arch/hppa/hppa/db_disasm.c,v
retrieving revision 1.20
diff -u -p -r1.20 db_disasm.c
--- arch/hppa/hppa/db_disasm.c 15 Oct 2014 17:22:56 -0000 1.20
+++ arch/hppa/hppa/db_disasm.c 5 Nov 2019 12:22:33 -0000
@@ -2309,9 +2309,7 @@ fmpyaddDasm(i, ofs, w)
 }
 
 vaddr_t
-db_disasm(loc, flag)
- vaddr_t loc;
- boolean_t flag;
+db_disasm(vaddr_t loc, int flag)
 {
  register const struct inst *i;
  register const struct majoropcode *m;
Index: arch/sparc64/include/db_machdep.h
===================================================================
RCS file: /cvs/src/sys/arch/sparc64/include/db_machdep.h,v
retrieving revision 1.19
diff -u -p -r1.19 db_machdep.h
--- arch/sparc64/include/db_machdep.h 23 Mar 2019 05:47:23 -0000 1.19
+++ arch/sparc64/include/db_machdep.h 5 Nov 2019 12:23:07 -0000
@@ -107,11 +107,11 @@ extern db_regs_t ddb_regs; /* register s
  */
 #define SOFTWARE_SSTEP
 
-boolean_t db_inst_trap_return(int inst);
-boolean_t db_inst_return(int inst);
-boolean_t db_inst_call(int inst);
-boolean_t db_inst_branch(int inst);
-boolean_t db_inst_unconditional_flow_transfer(int inst);
+int db_inst_trap_return(int inst);
+int db_inst_return(int inst);
+int db_inst_call(int inst);
+int db_inst_branch(int inst);
+int db_inst_unconditional_flow_transfer(int inst);
 db_addr_t db_branch_taken(int inst, db_addr_t pc, db_regs_t *regs);
 
 #define inst_trap_return(ins) db_inst_trap_return(ins)
Index: arch/sparc64/sparc64/db_disasm.c
===================================================================
RCS file: /cvs/src/sys/arch/sparc64/sparc64/db_disasm.c,v
retrieving revision 1.6
diff -u -p -r1.6 db_disasm.c
--- arch/sparc64/sparc64/db_disasm.c 15 Jan 2004 17:22:28 -0000 1.6
+++ arch/sparc64/sparc64/db_disasm.c 5 Nov 2019 12:23:33 -0000
@@ -874,9 +874,7 @@ struct sparc_insn sparc_i[] = {
 };
 
 db_addr_t
-db_disasm(loc, altfmt)
- vaddr_t loc;
- boolean_t altfmt;
+db_disasm(vaddr_t loc, int altfmt)
 {
  struct sparc_insn* i_ptr = (struct sparc_insn *)&sparc_i;
 
Index: arch/sparc64/sparc64/db_interface.c
===================================================================
RCS file: /cvs/src/sys/arch/sparc64/sparc64/db_interface.c,v
retrieving revision 1.52
diff -u -p -r1.52 db_interface.c
--- arch/sparc64/sparc64/db_interface.c 2 Aug 2019 02:17:35 -0000 1.52
+++ arch/sparc64/sparc64/db_interface.c 5 Nov 2019 12:25:03 -0000
@@ -65,7 +65,7 @@
 struct db_mutex ddb_mp_mutex = DB_MUTEX_INITIALIZER;
 volatile int ddb_state = DDB_STATE_NOT_RUNNING;
 volatile cpuid_t ddb_active_cpu;
-boolean_t db_switch_cpu;
+int db_switch_cpu;
 struct cpu_info *db_switch_to_cpu;
 #endif
 
@@ -299,13 +299,13 @@ db_ktrap(type, tf)
 
  s = splhigh();
  db_active++;
- cnpollc(TRUE);
+ cnpollc(1);
  /* Need to do spl stuff till cnpollc works */
  tl = ddb_regs.ddb_tl = savetstate(ts);
  db_dump_ts(0, 0, 0, 0);
  db_trap(type, 0/*code*/);
  restoretstate(tl,ts);
- cnpollc(FALSE);
+ cnpollc(0);
  db_active--;
  splx(s);
 
@@ -1269,9 +1269,8 @@ db_branch_taken(inst, pc, regs)
     }
 }
 
-boolean_t
-db_inst_branch(inst)
- int inst;
+int
+db_inst_branch(int inst)
 {
     union instr insn;
 
@@ -1280,13 +1279,13 @@ db_inst_branch(inst)
     /* the fancy union just gets in the way of this: */
     switch(inst & 0xffc00000) {
     case 0x30400000: /* branch always, annul, with prediction */
- return TRUE;
+ return 1;
     case 0x30800000: /* branch always, annul */
- return TRUE;
+ return 1;
     }
 
     if (insn.i_any.i_op != IOP_OP2)
- return FALSE;
+ return 0;
 
     switch (insn.i_op2.i_op2) {
       case IOP2_BPcc:
@@ -1295,17 +1294,16 @@ db_inst_branch(inst)
       case IOP2_FBPfcc:
       case IOP2_FBfcc:
       case IOP2_CBccc:
- return TRUE;
+ return 1;
 
       default:
- return FALSE;
+ return 0;
     }
 }
 
 
-boolean_t
-db_inst_call(inst)
- int inst;
+int
+db_inst_call(int inst)
 {
     union instr insn;
 
@@ -1313,30 +1311,29 @@ db_inst_call(inst)
 
     switch (insn.i_any.i_op) {
       case IOP_CALL:
- return TRUE;
+ return 1;
 
       case IOP_reg:
  return (insn.i_op3.i_op3 == IOP3_JMPL) && !db_inst_return(inst);
 
       default:
- return FALSE;
+ return 0;
     }
 }
 
 
-boolean_t
-db_inst_unconditional_flow_transfer(inst)
- int inst;
+int
+db_inst_unconditional_flow_transfer(int inst)
 {
     union instr insn;
 
     insn.i_int = inst;
 
     if (db_inst_call(inst))
- return TRUE;
+ return 1;
 
     if (insn.i_any.i_op != IOP_OP2)
- return FALSE;
+ return 0;
 
     switch (insn.i_op2.i_op2)
     {
@@ -1348,22 +1345,20 @@ db_inst_unconditional_flow_transfer(inst
  return insn.i_branch.i_cond == Icc_A;
 
       default:
- return FALSE;
+ return 0;
     }
 }
 
 
-boolean_t
-db_inst_return(inst)
- int inst;
+int
+db_inst_return(int inst)
 {
     return (inst == I_JMPLri(I_G0, I_O7, 8) || /* ret */
     inst == I_JMPLri(I_G0, I_I7, 8)); /* retl */
 }
 
-boolean_t
-db_inst_trap_return(inst)
- int inst;
+int
+db_inst_trap_return(int inst)
 {
     union instr insn;
 
Index: arch/sparc64/sparc64/db_trace.c
===================================================================
RCS file: /cvs/src/sys/arch/sparc64/sparc64/db_trace.c,v
retrieving revision 1.17
diff -u -p -r1.17 db_trace.c
--- arch/sparc64/sparc64/db_trace.c 1 Nov 2019 18:03:50 -0000 1.17
+++ arch/sparc64/sparc64/db_trace.c 5 Nov 2019 12:25:32 -0000
@@ -60,15 +60,15 @@ db_stack_trace_print(db_expr_t addr, int
     char *modif, int (*pr)(const char *, ...))
 {
  vaddr_t frame;
- boolean_t kernel_only = TRUE;
- boolean_t trace_thread = FALSE;
+ int kernel_only = 1;
+ int trace_thread = 0;
  char c, *cp = modif;
 
  while ((c = *cp++) != 0) {
  if (c == 't')
- trace_thread = TRUE;
+ trace_thread = 1;
  if (c == 'u')
- kernel_only = FALSE;
+ kernel_only = 0;
  }
 
  if (!have_addr)
@@ -275,12 +275,12 @@ db_dump_stack(db_expr_t addr, int have_a
 {
  int i;
  u_int64_t frame, oldframe;
- boolean_t kernel_only = TRUE;
+ int kernel_only = 1;
  char c, *cp = modif;
 
  while ((c = *cp++) != 0)
  if (c == 'u')
- kernel_only = FALSE;
+ kernel_only = 0;
 
  if (count == -1)
  count = 65535;

Reply | Threaded
Open this post in threaded view
|

Re: un-boolean_t ANSIfy ddb(4) for hppa & sparc64

Andras Farkas
Out of curiosity, why not just use stdbool.h if these are actually
meant to be booleans?  Wouldn't that be more readable?
I wonder if there's something I'm not understanding.

On Tue, Nov 5, 2019 at 8:05 AM Martin Pieuchot <[hidden email]> wrote:

>
> ok?
>
> Index: arch/hppa/hppa/db_disasm.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/hppa/db_disasm.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 db_disasm.c
> --- arch/hppa/hppa/db_disasm.c  15 Oct 2014 17:22:56 -0000      1.20
> +++ arch/hppa/hppa/db_disasm.c  5 Nov 2019 12:22:33 -0000
> [snip]

Reply | Threaded
Open this post in threaded view
|

Re: un-boolean_t ANSIfy ddb(4) for hppa & sparc64

Theo de Raadt-2
Kernel environment cannot use userland includes.

Andras Farkas <[hidden email]> wrote:

> Out of curiosity, why not just use stdbool.h if these are actually
> meant to be booleans?  Wouldn't that be more readable?
> I wonder if there's something I'm not understanding.
>
> On Tue, Nov 5, 2019 at 8:05 AM Martin Pieuchot <[hidden email]> wrote:
> >
> > ok?
> >
> > Index: arch/hppa/hppa/db_disasm.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/hppa/hppa/db_disasm.c,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 db_disasm.c
> > --- arch/hppa/hppa/db_disasm.c  15 Oct 2014 17:22:56 -0000      1.20
> > +++ arch/hppa/hppa/db_disasm.c  5 Nov 2019 12:22:33 -0000
> > [snip]
>

Reply | Threaded
Open this post in threaded view
|

Re: un-boolean_t ANSIfy ddb(4) for hppa & sparc64

Todd C. Miller-3
On Wed, 20 Nov 2019 07:38:46 -0700, "Theo de Raadt" wrote:

> Kernel environment cannot use userland includes.

Some other systems have sys/stdbool.h, we could as well if we wanted
to.  The simplest approach is to move include/stdbool.h ->
sys/sys/stdbool.h and make /usr/include/stdbool.h a link to
sys/stdbool.h as we do for stdarg.h and stdint.h.

 - todd

Reply | Threaded
Open this post in threaded view
|

Re: un-boolean_t ANSIfy ddb(4) for hppa & sparc64

Theo de Raadt-2
Todd C. Miller <[hidden email]> wrote:

> On Wed, 20 Nov 2019 07:38:46 -0700, "Theo de Raadt" wrote:
>
> > Kernel environment cannot use userland includes.
>
> Some other systems have sys/stdbool.h, we could as well if we wanted
> to.  The simplest approach is to move include/stdbool.h ->
> sys/sys/stdbool.h and make /usr/include/stdbool.h a link to
> sys/stdbool.h as we do for stdarg.h and stdint.h.

But is it really neccessary?  Has int really caused everyone that
much harm?

Reply | Threaded
Open this post in threaded view
|

Re: un-boolean_t ANSIfy ddb(4) for hppa & sparc64

Todd C. Miller-3
In reply to this post by Andras Farkas
On Wed, 20 Nov 2019 05:42:39 -0500, Andras Farkas wrote:

> Out of curiosity, why not just use stdbool.h if these are actually
> meant to be booleans?  Wouldn't that be more readable?
> I wonder if there's something I'm not understanding.

Changing from int -> bool is error prone since you inevitably have
code that use true, false and -1.  Finding those outliers that are
relying on the type being int is not always easy.  Speaking from
personal experiance, it is easy to introduce bugs this way.

 - todd

Reply | Threaded
Open this post in threaded view
|

Re: un-boolean_t ANSIfy ddb(4) for hppa & sparc64

Todd C. Miller-3
In reply to this post by Theo de Raadt-2
On Wed, 20 Nov 2019 10:17:09 -0700, "Theo de Raadt" wrote:

> Todd C. Miller <[hidden email]> wrote:
>
> > On Wed, 20 Nov 2019 07:38:46 -0700, "Theo de Raadt" wrote:
> >
> > > Kernel environment cannot use userland includes.
> >
> > Some other systems have sys/stdbool.h, we could as well if we wanted
> > to.  The simplest approach is to move include/stdbool.h ->
> > sys/sys/stdbool.h and make /usr/include/stdbool.h a link to
> > sys/stdbool.h as we do for stdarg.h and stdint.h.
>
> But is it really neccessary?  Has int really caused everyone that
> much harm?

From a readability standpoint, it is nice to be able to use true
and false.  You can use those with int as well as with bool.

 - todd

Reply | Threaded
Open this post in threaded view
|

Re: un-boolean_t ANSIfy ddb(4) for hppa & sparc64

Theo de Raadt-2
Todd C. Miller <[hidden email]> wrote:

> On Wed, 20 Nov 2019 10:17:09 -0700, "Theo de Raadt" wrote:
>
> > Todd C. Miller <[hidden email]> wrote:
> >
> > > On Wed, 20 Nov 2019 07:38:46 -0700, "Theo de Raadt" wrote:
> > >
> > > > Kernel environment cannot use userland includes.
> > >
> > > Some other systems have sys/stdbool.h, we could as well if we wanted
> > > to.  The simplest approach is to move include/stdbool.h ->
> > > sys/sys/stdbool.h and make /usr/include/stdbool.h a link to
> > > sys/stdbool.h as we do for stdarg.h and stdint.h.
> >
> > But is it really neccessary?  Has int really caused everyone that
> > much harm?
>
> From a readability standpoint, it is nice to be able to use true
> and false.  You can use those with int as well as with bool.

I don't think the type of readability stdbool.h brings results in 1
fewer bug in the resulting code, also I think adding it to a body of
code late risks introducing bugs.

More to the point, whenever I see codebases which mixes true/false,
0/-1, and 0/1, I don't pick up a vibe of "better readability".

And it gets even worse when one see that stdbool.h has 3 variations
internally which are not 100.0% compatible.

I don't see any value bringing it to the kernel.

Reply | Threaded
Open this post in threaded view
|

Re: un-boolean_t ANSIfy ddb(4) for hppa & sparc64

Jonathan Gray-11
On Wed, Nov 20, 2019 at 10:25:59AM -0700, Theo de Raadt wrote:

> Todd C. Miller <[hidden email]> wrote:
>
> > On Wed, 20 Nov 2019 10:17:09 -0700, "Theo de Raadt" wrote:
> >
> > > Todd C. Miller <[hidden email]> wrote:
> > >
> > > > On Wed, 20 Nov 2019 07:38:46 -0700, "Theo de Raadt" wrote:
> > > >
> > > > > Kernel environment cannot use userland includes.
> > > >
> > > > Some other systems have sys/stdbool.h, we could as well if we wanted
> > > > to.  The simplest approach is to move include/stdbool.h ->
> > > > sys/sys/stdbool.h and make /usr/include/stdbool.h a link to
> > > > sys/stdbool.h as we do for stdarg.h and stdint.h.
> > >
> > > But is it really neccessary?  Has int really caused everyone that
> > > much harm?
> >
> > From a readability standpoint, it is nice to be able to use true
> > and false.  You can use those with int as well as with bool.
>
> I don't think the type of readability stdbool.h brings results in 1
> fewer bug in the resulting code, also I think adding it to a body of
> code late risks introducing bugs.
>
> More to the point, whenever I see codebases which mixes true/false,
> 0/-1, and 0/1, I don't pick up a vibe of "better readability".
>
> And it gets even worse when one see that stdbool.h has 3 variations
> internally which are not 100.0% compatible.
>
> I don't see any value bringing it to the kernel.

bool in the kernel is covered by /sys/sys/types.h not stdbool.h but is
really only for drm

revision 1.35
date: 2013/01/09 12:17:38;  author: jsg;  state: Exp;  lines: +28 -1;
add support for using c99 bool in the kernel based on our stdbool.h
ok deraadt@ millert@ espie@

Reply | Threaded
Open this post in threaded view
|

Re: un-boolean_t ANSIfy ddb(4) for hppa & sparc64

Andras Farkas
In reply to this post by Theo de Raadt-2
On Wed, Nov 20, 2019 at 9:38 AM Theo de Raadt <[hidden email]> wrote:
> Kernel environment cannot use userland includes.

On Wed, Nov 20, 2019 at 12:18 PM Todd C. Miller <[hidden email]> wrote:
> Changing from int -> bool is error prone since you inevitably have
> code that use true, false and -1.  Finding those outliers that are
> relying on the type being int is not always easy.  Speaking from
> personal experiance, it is easy to introduce bugs this way.

I see. Thanks for the info!
:D