mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Jens Gustedt <jens.gustedt@inria.fr>
Cc: musl@lists.openwall.com
Subject: Re: alternative form flag with zero octal value
Date: Fri, 12 Jan 2018 09:54:23 +0100	[thread overview]
Message-ID: <20180112095423.09695102@inria.fr> (raw)
In-Reply-To: <20180112003811.GJ1627@brightrain.aerifal.cx>

[-- Attachment #1: Type: text/plain, Size: 4685 bytes --]

Hello Rich,

On Thu, 11 Jan 2018 19:38:11 -0500 Rich Felker <dalias@libc.org> wrote:

> I'm not sure we're on the same page about what's bad then. I think
> 
> 		if (1) something; else
> 	case bar:
> 		something_else;
> 
> is gratuitously less readable than:
> 
> 		something;
> 		goto shared_tail;
> 	case bar:
> 		something_else;
> 	shared_tail:

Depends, I think non-case labels are disruptive if there are too many
in the same function. A more clearly structured code, but with
compound { } would also clearly tell where the flow goes.

> Ideally we wouldn't need to do either, but sometimes it's hard to
> organized the code in a way that's not gratuitously redundant (at the
> source level) or gratuitously larger without stuff like this.

sure

> > > or compound statatements and
> > > probably should be. I don't want a big patch for this file that's
> > > hard to review/validate though; the return on time spent is just
> > > too small. If anyone does want to push me on making improvements
> > > here, small isolated/individual changes that can easily be seen
> > > to be correct are the way to go.  
> > 
> > Just a patch for white space and indentation would be easy, I
> > think.  
> 
> I'm not a fan of patches that just recreate the output of an indention
> utility.
> 
> >      - have break at the end of case lines, not at the start  
> 
> I guess you mean the pop_arg function? It's written the way it is
> because that's the form that makes it the easiest to see what's
> happening in each case and what's different between them. Putting the
> break on a separate line would make it take up a lot more space and
> lower the visual SNR. Putting it at the end of the line (without a lot
> of alignment space; such space could be an alternative option) would
> butt rendundant text up against the part that differs, making it
> harder to read too.
> 
> One appproach that might satisfy us both is using a macro, something
> like:
> 
> #define POP_CASE(a,b,c) case a: arg->b = va_arg(*ap, c); break;
> 
> obviously with more meaningful names for the args. Then each line of
> the switch only contains meaningful information.
> 
> >      - start a newline before else
> >      - ident according to musl's coding style  
> 
> I think a misunderstanding/different expectation you and perhaps
> others have is that a "coding style" is a deterministic function from
> source token sequences (and possibly some additional metadata like
> presence of blank lines) to a canonical source file. Some projects do
> things this way, but it's never been something I've liked. To me,
> "coding style" means a set of formatting constraints that should
> usually be met, unless there's a good reason to break them, but which
> don't determine a unique form, plus guidelines for how to favor
> readability when there's ambiguity about the best form.

For the grammatical part of "coding style" I effectively see it as
more or less mechanical. Allowing for exceptions is not helping the
occasional reader who at the same time has to apprehend the cleverness
of the layout and of the code itself. This vfprintf code is an
excellent example of that point. Just indenting it with the correct
tabs makes it better readable for me.

> > As long as git diff --color-words shows nothing, this should be
> > fine.
> > 
> > I could such a patch, if you want.  
> 
> I'd really rather spend time reviewing & merging functional patches
> and implementing new things than discussing coding style. As I said
> before, if there are individual/isolated changes that can be accepted
> or deferred or rejected without requiring a long thread of v2, v3,
> ..., v10 patches until we agree on which combination of changes is
> right, I'm happy to commit the ones I agree with right away, but I'd
> still rather be spending time on something that feels more productive.
> 
> > Replacing comma operators by compound statements would be a bit more
> > work.  We could delay that to when somebody touches that file for
> > more serious reasons.  
> 
> Perhaps. Sometimes I'm not sure whether I prefer changing this sort of
> thing at the same time as a functional patch or doing it preemptively
> so that later functional changes are slimmer and easier to understand.

Let's forget about this then.

Jens

-- 
:: INRIA Nancy Grand Est ::: Camus ::::::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

      reply	other threads:[~2018-01-12  8:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-11  0:43 Peter Wang
2018-01-11  2:02 ` Rich Felker
2018-01-11  2:17   ` Rich Felker
2018-01-11  8:12     ` Jens Gustedt
2018-01-11 18:55       ` Rich Felker
2018-01-11 19:36         ` Jens Gustedt
2018-01-12  0:38           ` Rich Felker
2018-01-12  8:54             ` Jens Gustedt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180112095423.09695102@inria.fr \
    --to=jens.gustedt@inria.fr \
    --cc=musl@lists.openwall.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).