mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Antony Pavlov <antonynpavlov@gmail.com>
To: Daniele Lacamera <daniele.lacamera@tass.be>
Cc: barebox <barebox@lists.infradead.org>,
	Sam Van Den Berge <sam.van.den.berge@tass.be>
Subject: Re: [RFC] [WIP] incorporate picotcp into barebox: a small demo
Date: Thu, 29 May 2014 09:40:02 +0400	[thread overview]
Message-ID: <20140529094002.696353df539fc4b2e73f383f@gmail.com> (raw)
In-Reply-To: <CAOngqVWfbkP7GunG25VP4-2a1_3y7Z0hxmBZf3OWKnL91b4JJA@mail.gmail.com>

On Tue, 27 May 2014 19:26:14 +0200
Daniele Lacamera <daniele.lacamera@tass.be> wrote:

> Antony, thank you very much for the comments and the bug reports on
> our github. See comments below.
> 
> On Tue, May 27, 2014 at 4:04 PM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > On Tue, 27 May 2014 11:46:29 +0200
> > Daniele Lacamera <daniele.lacamera@tass.be> wrote:
> >
> >
> > linux kernel and barebox use not so much #ifdef's because they use
> > they use IS_ENABLED() macro with just the same result.
> [...]
> > so the code under IS_ENABLED() is always parsed by C compiler;
> > if there is an syntax error in this code then the compilation will be stopped.
> 
> We will discuss this and see if it is applicable for PicoTCP. The
> macro you are suggesting is indeed not increasing code size, but our
> static checkers might be not so happy about all this preprocessor
> machinery.
> 
> >
> > $ gcc --version
> > gcc (Debian 4.8.3-1) 4.8.3
> >
> > Here is my 'make -s' output for sandbox barebox:
> >
> [cut]
> 
> Thank You!! This *IS* very helpful input. We are going to add more -W
> flags and quickly fix these warnings.
> 
> > But in linux kernel, barebox, qemu and IMHO many other projects
> > there are:
> >
> >   * CODING_STYLE documentation file;
> >   * scripts/checkpatch.pl
> >
> 
> All good points. Being a kernel developer myself, I know the
> checkpatch.pl approach, and indeed it is a good thing to distribute
> coding rules if you accept contributions. Except it will never apply
> to us, because:
> 
> - We have our company internal coding style which we apply to all our
> projects and we don't feel the need to distribute externally (keep in
> mind that we never planned to accept external contributions). Instead,
> the repository contains the uncrustify configuration file, which is
> pretty much self-explanatory.
> - A few of us are checking every single commit, and running uncrustify
> every now and then. We are comfortable with this approach

Be very carefull with this approach.

It looks like uncrustify changes not only whitespaces and newline symbols!
I suppose that it can change code logic!

E.g. please see this commit

commit 81f52a4ad8fd31edcedd7c91945c801899153d36
Author: Daniele Lacamera <daniele.lacamera@tass.be>
Date:   Wed Mar 12 12:29:21 2014 +0100

    Enforced coding style

Here is a notable fragment

--- a/modules/pico_mm.c
+++ b/modules/pico_mm.c
@@ -75,80 +79,80 @@ typedef union block_internals           block_internals;
         <---------------------------------------------------------------------->
 
 
-        +------------<------------+----------<-----------+
-        |                         ^                      ^
+   +------------<------------+----------<-----------+
+ |                         ^                      ^
         v                         |                      |
-        +---------+------------+--+----+---------------+-+-----+---------------+
-        |         |            |       |               |       |               |
-        |  pico_  |            | pico_ |               | pico_ |               |
-        |  mem_   | ...HEAP... | mem_  |     slab      | mem_  |     slab      |
-        |  page   |            | block |               | block |               |
-        |         |            |       |               |       |               |
-        +---------+------------+-------+-----+---------+-------+----------+----+
+ |+---------+------------+--+----+---------------+-+-----+---------------+
+ |         |            |       |               |       |               |
+ |  pico_  |            | pico_ |               | pico_ |               |
+ |  mem_   | ...HEAP... | mem_  |     slab      | mem_  |     slab      |
+ |  page   |            | block |               | block |               |
+ |         |            |       |               |       |               |
+ |+---------+------------+-------+-----+---------+-------+----------+----+

The number of '|' symbols is changed! After this commit you have at least
two additional '|' symbols. TIP: look at the start of the last line of the fragment.


> - Linux coding rules are generally recognized as possibly not the best
> in the world. I especially dislike tabs.
> - commit hooks are tedious
> 
> I am afraid we are not going to change our formatting style or policy,
> but thanks anyway for the suggestions!

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  reply	other threads:[~2014-05-29  5:29 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-25  9:58 Antony Pavlov
2014-05-26  9:35 ` Daniele Lacamera
2014-05-26  9:45 ` Lucas Stach
2014-05-26 12:09   ` Antony Pavlov
2014-05-27  5:30     ` Sascha Hauer
2014-05-27  7:52       ` Daniele Lacamera
2014-05-27  9:46     ` Daniele Lacamera
2014-05-27 14:04       ` Antony Pavlov
2014-05-27 17:26         ` Daniele Lacamera
2014-05-29  5:40           ` Antony Pavlov [this message]
2014-05-28  6:08         ` Sascha Hauer
2014-05-28  7:23         ` Juergen Borleis
2014-05-28 10:32           ` Antony Pavlov

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=20140529094002.696353df539fc4b2e73f383f@gmail.com \
    --to=antonynpavlov@gmail.com \
    --cc=barebox@lists.infradead.org \
    --cc=daniele.lacamera@tass.be \
    --cc=sam.van.den.berge@tass.be \
    /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.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox