Re: MP-safe /dev/console and /dev/constty

2022-10-02 Thread Mouse
>> Your suggestion of pushing it into a separate function (which
>> presumably would just mean using return instead of break to
>> terminate the code block) strikes me as worth considering in general
>> but a bad idea in this case; there are too many things that would
>> have to be passed down to the function in question.
> Of course, GCC offers nested functions for exactly this, but...

Yes.  I would not expect gcc-specific code to be accepted.

In this case, I see no benefit to using a nested function over one of
the constructs that supports a break-out to a well-defined point:
do{...}while(0), switch(0){case 0:...}, while(1){...;break;}, or the
like.  (I would say do-while(0) is the closest to a canonical version
of those.)  In some cases there may be a benefit, if you want to break
out of multiple nested constructs.  (In that case I'd actually use
labeled control structure, but that's even less well supported than
gccisms like nested functions.)

However, this is all armchair quarterbacking when we don't know what
mrg disliked about the code as given.  I still think all it really
needs is to be reformatted so the do and the while(0) don't visually
disappear into the containing if-elseif construct.

/~\ The ASCII Mouse
\ / Ribbon Campaign
 X  Against HTMLmo...@rodents-montreal.org
/ \ Email!   7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B


Re: MP-safe /dev/console and /dev/constty

2022-10-02 Thread Thor Lancelot Simon
On Sat, Oct 01, 2022 at 07:59:23PM -0400, Mouse wrote:
>
> usual approach to such things.  Your suggestion of pushing it into a
> separate function (which presumably would just mean using return
> instead of break to terminate the code block) strikes me as worth
> considering in general but a bad idea in this case; there are too many
> things that would have to be passed down to the function in question.

Of course, GCC offers nested functions for exactly this, but...

Thor


Re: MP-safe /dev/console and /dev/constty

2022-10-01 Thread Mouse
> i really like this except for the if () do { ... } while (0); else
> abuse portion.  please rework that part.  it looks easiest to push
> into a separate function, perhaps.

You don't say what you don't like about it.

There are only two things I don't like about it, and one of them
(indentation) is shared with almost all of /usr/src.  (The other I'll
get to below.)  Given the lack of information about what you don't like
about it, I'm going to guess that you don't like using an un-braced
do-while as the consequent of an if.  Or, perhaps, you don't like that
use of do-while at all?

Using do { ... } while (0); to provide a context in which break can be
used to skip the rest of a well-defined block of code is, IMO, far
preferable to using a goto, which latter seems to be the historically
usual approach to such things.  Your suggestion of pushing it into a
separate function (which presumably would just mean using return
instead of break to terminate the code block) strikes me as worth
considering in general but a bad idea in this case; there are too many
things that would have to be passed down to the function in question.
And the only benefit I see is avoiding the do-while, which I have
trouble seeing anything wrong with, except the second of the two things
I mentioend above.

Would you feel better if it were wrapped in switch (0) { case 0: ... }
instead?  Worse?  Why or why not?

I would prefer to see braces around the do-while, with a corresponding
indentation level, but that's the only change I would say needs making
there.  With the current formatting, the do and while(0) tend to
visually disappear into the if control structure, making the contained
breaks too easy to misread.

/~\ The ASCII Mouse
\ / Ribbon Campaign
 X  Against HTMLmo...@rodents-montreal.org
/ \ Email!   7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B


re: MP-safe /dev/console and /dev/constty

2022-10-01 Thread matthew green
i really like this except for the if () do { ... } while (0); else
abuse portion.  please rework that part.  it looks easiest to push
into a separate function, perhaps.

thanks.


.mrg.