Warnings do include offending filename

Warnings do include offending filename

[Ian Campbell]

Hello,

I wasn't sure how/where to make a wishlist bug report, so I hope this
will suffice, am happy to be pointed in a different direction though.

I recently[0] stumbled over around 1,000 of these:
    Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
    Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
    Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
    Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
    Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
    Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
    Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name

When building the split device tree repo[1] from the Linux source
(essential it's a build of every single dts in the kernel source).

The cause of the warning is an issue which needs to be fixed but I
thought I would mention that it would be very useful (I expect) if dtc
would include the offending file in warnings (like e.g. gcc would), not
just because of the number of *.dtb being built here but also due to
#include and /include/ of .dtsi files.

Thanks,
Ian.

[0] <1485765782.7612.21.camel-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org> on devicetree@vger at
Mon, 30 Jan 2017 08:43:02 +0000, it still hasn't hit the archives it
seems.
[1] https://git.kernel.org/cgit/linux/kernel/git/devicetree/devicetree-rebasing.git/

Re: Warnings do include offending filename

[Ian Campbell]

On Mon, 2017-01-30 at 09:13 +0000, Ian Campbell wrote:

> [0] <1485765782.7612.21.camel-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org> on devicetree@vger at
> Mon, 30 Jan 2017 08:43:02 +0000, it still hasn't hit the archives it
> seems.

This didn't make it through due to XXX in my original subject, http://m
arc.info/?l=devicetree&m=148577021420939&w=2 is the resend.

Ian.

Re: Warnings do include offending filename

[David Gibson]

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

On Mon, Jan 30, 2017 at 09:13:05AM +0000, Ian Campbell wrote:
> Hello,
> 
> I wasn't sure how/where to make a wishlist bug report, so I hope this
> will suffice, am happy to be pointed in a different direction though.
> 
> I recently[0] stumbled over around 1,000 of these:
>     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
>     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
>     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
>     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
>     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
>     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
>     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> 
> When building the split device tree repo[1] from the Linux source
> (essential it's a build of every single dts in the kernel source).
> 
> The cause of the warning is an issue which needs to be fixed but I
> thought I would mention that it would be very useful (I expect) if dtc
> would include the offending file in warnings (like e.g. gcc would), not
> just because of the number of *.dtb being built here but also due to
> #include and /include/ of .dtsi files.

Right, having the filenames - and line numbers - there would certainly
be helpful.  Unfortunately, it's not at all trivial to implement.  As
someone said in a different thread, these checks take place (and have
to) after the tree is completely parsed and we no longer have source
locations readily to hand.

It might be possible to attach (optional) srcpos * markers to each
node and property in the live tree.  However, that runs into other
complications: when using dts includes nodes can be merges of
information from several different dtsi files.  So, a node can be
composed of information from several non-contiguous chunks of source
from different files.  It's not clear what should be printed in that
case.

If you can see a reasonable way to do this, I'd love to see patches
for it, but I certainly don't have time to tackle it myself.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Warnings do include offending filename

[Ian Campbell]

On Tue, 2017-01-31 at 10:49 +1100, David Gibson wrote:
> On Mon, Jan 30, 2017 at 09:13:05AM +0000, Ian Campbell wrote:
> > Hello,
> > 
> > I wasn't sure how/where to make a wishlist bug report, so I hope this
> > will suffice, am happy to be pointed in a different direction though.
> > 
> > I recently[0] stumbled over around 1,000 of these:
> >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > 
> > When building the split device tree repo[1] from the Linux source
> > (essential it's a build of every single dts in the kernel source).
> > 
> > The cause of the warning is an issue which needs to be fixed but I
> > thought I would mention that it would be very useful (I expect) if dtc
> > would include the offending file in warnings (like e.g. gcc would), not
> > just because of the number of *.dtb being built here but also due to
> > #include and /include/ of .dtsi files.
> 
> Right, having the filenames - and line numbers - there would certainly
> be helpful.  Unfortunately, it's not at all trivial to implement.  As
> someone said in a different thread, these checks take place (and have
> to) after the tree is completely parsed and we no longer have source
> locations readily to hand.

Would it be easier (or possible) to print the name of the eventually-
to-be-output binary? At the moment the user is left guessing which one
of 1,200 *.dtb files they just built produced each of the similar
number of warnings. If the message was instead:

    Warning (unit_address_vs_reg): arch/arm/boot/dts/foo.dtb: Node /soc has a ...

Then that would at least be something to go on.

In fact, given the checks are on the final tree, naming the output file
 in the messages seems fairly logical (you could even imagine doing
these checks in a separate linter tool after the fact, given the *.dtb
as input, I suppose)

> It might be possible to attach (optional) srcpos * markers to each
> node and property in the live tree.  However, that runs into other
> complications: when using dts includes nodes can be merges of
> information from several different dtsi files.  So, a node can be
> composed of information from several non-contiguous chunks of source
> from different files.  It's not clear what should be printed in that
> case.

Yeah, tricky!

Hopefully the above suggesting will suffice for 99% of cases.

Ian

Re: Warnings do include offending filename

[David Gibson]

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

On Tue, Jan 31, 2017 at 08:24:48AM +0000, Ian Campbell wrote:
> On Tue, 2017-01-31 at 10:49 +1100, David Gibson wrote:
> > On Mon, Jan 30, 2017 at 09:13:05AM +0000, Ian Campbell wrote:
> > > Hello,
> > > 
> > > I wasn't sure how/where to make a wishlist bug report, so I hope this
> > > will suffice, am happy to be pointed in a different direction though.
> > > 
> > > I recently[0] stumbled over around 1,000 of these:
> > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > > 
> > > When building the split device tree repo[1] from the Linux source
> > > (essential it's a build of every single dts in the kernel source).
> > > 
> > > The cause of the warning is an issue which needs to be fixed but I
> > > thought I would mention that it would be very useful (I expect) if dtc
> > > would include the offending file in warnings (like e.g. gcc would), not
> > > just because of the number of *.dtb being built here but also due to
> > > #include and /include/ of .dtsi files.
> > 
> > Right, having the filenames - and line numbers - there would certainly
> > be helpful.  Unfortunately, it's not at all trivial to implement.  As
> > someone said in a different thread, these checks take place (and have
> > to) after the tree is completely parsed and we no longer have source
> > locations readily to hand.
> 
> Would it be easier (or possible) to print the name of the eventually-
> to-be-output binary? At the moment the user is left guessing which one
> of 1,200 *.dtb files they just built produced each of the similar
> number of warnings. If the message was instead:
> 
>     Warning (unit_address_vs_reg): arch/arm/boot/dts/foo.dtb: Node /soc has a ...
> 
> Then that would at least be something to go on.
> 
> In fact, given the checks are on the final tree, naming the output file
>  in the messages seems fairly logical (you could even imagine doing
> these checks in a separate linter tool after the fact, given the *.dtb
> as input, I suppose)

Hm, possible, though a bit messy to do within dtc.  The output file is
currently passed into that section of the code, but I guess we could
add it.

However, it seems this would more easily be fixed from the Makefile
side: if you echo a (suitably abbreviated) dtc command line, then it
should become obvious which dtb the errors are associated with.

> > It might be possible to attach (optional) srcpos * markers to each
> > node and property in the live tree.  However, that runs into other
> > complications: when using dts includes nodes can be merges of
> > information from several different dtsi files.  So, a node can be
> > composed of information from several non-contiguous chunks of source
> > from different files.  It's not clear what should be printed in that
> > case.
> 
> Yeah, tricky!
> 
> Hopefully the above suggesting will suffice for 99% of cases.

Actually, this might be made more feasible by some of the overlay
stuff in the works.  We're looking at doing the overlay merges in a
separate pass after parsing is complete.  There are some interesting
edge cases about whether checks get run before or after that merge,
but I'm hoping most of the checks can run before.  For those that do,
we should have a well defined chunk of source from which each node and
property comes.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Warnings do include offending filename

[David Gibson]

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

On Wed, Feb 01, 2017 at 11:16:54AM +1100, David Gibson wrote:
> On Tue, Jan 31, 2017 at 08:24:48AM +0000, Ian Campbell wrote:
> > On Tue, 2017-01-31 at 10:49 +1100, David Gibson wrote:
> > > On Mon, Jan 30, 2017 at 09:13:05AM +0000, Ian Campbell wrote:
> > > > Hello,
> > > > 
> > > > I wasn't sure how/where to make a wishlist bug report, so I hope this
> > > > will suffice, am happy to be pointed in a different direction though.
> > > > 
> > > > I recently[0] stumbled over around 1,000 of these:
> > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > > > 
> > > > When building the split device tree repo[1] from the Linux source
> > > > (essential it's a build of every single dts in the kernel source).
> > > > 
> > > > The cause of the warning is an issue which needs to be fixed but I
> > > > thought I would mention that it would be very useful (I expect) if dtc
> > > > would include the offending file in warnings (like e.g. gcc would), not
> > > > just because of the number of *.dtb being built here but also due to
> > > > #include and /include/ of .dtsi files.
> > > 
> > > Right, having the filenames - and line numbers - there would certainly
> > > be helpful.  Unfortunately, it's not at all trivial to implement.  As
> > > someone said in a different thread, these checks take place (and have
> > > to) after the tree is completely parsed and we no longer have source
> > > locations readily to hand.
> > 
> > Would it be easier (or possible) to print the name of the eventually-
> > to-be-output binary? At the moment the user is left guessing which one
> > of 1,200 *.dtb files they just built produced each of the similar
> > number of warnings. If the message was instead:
> > 
> >     Warning (unit_address_vs_reg): arch/arm/boot/dts/foo.dtb: Node /soc has a ...
> > 
> > Then that would at least be something to go on.
> > 
> > In fact, given the checks are on the final tree, naming the output file
> >  in the messages seems fairly logical (you could even imagine doing
> > these checks in a separate linter tool after the fact, given the *.dtb
> > as input, I suppose)
> 
> Hm, possible, though a bit messy to do within dtc.  The output file is

s/is/isn't/  duh.

> currently passed into that section of the code, but I guess we could
> add it.
> 
> However, it seems this would more easily be fixed from the Makefile
> side: if you echo a (suitably abbreviated) dtc command line, then it
> should become obvious which dtb the errors are associated with.
> 
> > > It might be possible to attach (optional) srcpos * markers to each
> > > node and property in the live tree.  However, that runs into other
> > > complications: when using dts includes nodes can be merges of
> > > information from several different dtsi files.  So, a node can be
> > > composed of information from several non-contiguous chunks of source
> > > from different files.  It's not clear what should be printed in that
> > > case.
> > 
> > Yeah, tricky!
> > 
> > Hopefully the above suggesting will suffice for 99% of cases.
> 
> Actually, this might be made more feasible by some of the overlay
> stuff in the works.  We're looking at doing the overlay merges in a
> separate pass after parsing is complete.  There are some interesting
> edge cases about whether checks get run before or after that merge,
> but I'm hoping most of the checks can run before.  For those that do,
> we should have a well defined chunk of source from which each node and
> property comes.
> 



-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Warnings do include offending filename

[Ian Campbell]

On Wed, 2017-02-01 at 12:00 +1100, David Gibson wrote:
> On Wed, Feb 01, 2017 at 11:16:54AM +1100, David Gibson wrote:
> > On Tue, Jan 31, 2017 at 08:24:48AM +0000, Ian Campbell wrote:
> > > On Tue, 2017-01-31 at 10:49 +1100, David Gibson wrote:
> > > > On Mon, Jan 30, 2017 at 09:13:05AM +0000, Ian Campbell wrote:
> > > > > Hello,
> > > > > 
> > > > > I wasn't sure how/where to make a wishlist bug report, so I hope this
> > > > > will suffice, am happy to be pointed in a different direction though.
> > > > > 
> > > > > I recently[0] stumbled over around 1,000 of these:
> > > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > > > > 
> > > > > When building the split device tree repo[1] from the Linux source
> > > > > (essential it's a build of every single dts in the kernel source).
> > > > > 
> > > > > The cause of the warning is an issue which needs to be fixed but I
> > > > > thought I would mention that it would be very useful (I expect) if dtc
> > > > > would include the offending file in warnings (like e.g. gcc would), not
> > > > > just because of the number of *.dtb being built here but also due to
> > > > > #include and /include/ of .dtsi files.
> > > > 
> > > > Right, having the filenames - and line numbers - there would certainly
> > > > be helpful.  Unfortunately, it's not at all trivial to implement.  As
> > > > someone said in a different thread, these checks take place (and have
> > > > to) after the tree is completely parsed and we no longer have source
> > > > locations readily to hand.
> > > 
> > > Would it be easier (or possible) to print the name of the eventually-
> > > to-be-output binary? At the moment the user is left guessing which one
> > > of 1,200 *.dtb files they just built produced each of the similar
> > > number of warnings. If the message was instead:
> > > 
> > >     Warning (unit_address_vs_reg): arch/arm/boot/dts/foo.dtb: Node /soc has a ...
> > > 
> > > Then that would at least be something to go on.
> > > 
> > > In fact, given the checks are on the final tree, naming the output file
> > >  in the messages seems fairly logical (you could even imagine doing
> > > these checks in a separate linter tool after the fact, given the *.dtb
> > > as input, I suppose)
> > 
> > Hm, possible, though a bit messy to do within dtc.  The output file is
> 
> s/is/isn't/  duh.
> 
> > currently passed into that section of the code, but I guess we could
> > add it.
> > 
> > However, it seems this would more easily be fixed from the Makefile
> > side: if you echo a (suitably abbreviated) dtc command line, then it
> > should become obvious which dtb the errors are associated with.

Not with "make -j12" sadly. I suppose people could rerun make without
the -j to figure out where the warning came from, but it seems rather
suboptimal to me.

Ian.

Re: Warnings do include offending filename

[David Gibson]

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

On Wed, Feb 01, 2017 at 07:34:06AM +0000, Ian Campbell wrote:
> On Wed, 2017-02-01 at 12:00 +1100, David Gibson wrote:
> > On Wed, Feb 01, 2017 at 11:16:54AM +1100, David Gibson wrote:
> > > On Tue, Jan 31, 2017 at 08:24:48AM +0000, Ian Campbell wrote:
> > > > On Tue, 2017-01-31 at 10:49 +1100, David Gibson wrote:
> > > > > On Mon, Jan 30, 2017 at 09:13:05AM +0000, Ian Campbell wrote:
> > > > > > Hello,
> > > > > > 
> > > > > > I wasn't sure how/where to make a wishlist bug report, so I hope this
> > > > > > will suffice, am happy to be pointed in a different direction though.
> > > > > > 
> > > > > > I recently[0] stumbled over around 1,000 of these:
> > > > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > > > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > > > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > > > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > > > > >     Warning (unit_address_vs_reg): Node /soc has a reg or ranges property, but no unit name
> > > > > >     Warning (unit_address_vs_reg): Node /soc/main-oscillator has a reg or ranges property, but no unit name
> > > > > > 
> > > > > > When building the split device tree repo[1] from the Linux source
> > > > > > (essential it's a build of every single dts in the kernel source).
> > > > > > 
> > > > > > The cause of the warning is an issue which needs to be fixed but I
> > > > > > thought I would mention that it would be very useful (I expect) if dtc
> > > > > > would include the offending file in warnings (like e.g. gcc would), not
> > > > > > just because of the number of *.dtb being built here but also due to
> > > > > > #include and /include/ of .dtsi files.
> > > > > 
> > > > > Right, having the filenames - and line numbers - there would certainly
> > > > > be helpful.  Unfortunately, it's not at all trivial to implement.  As
> > > > > someone said in a different thread, these checks take place (and have
> > > > > to) after the tree is completely parsed and we no longer have source
> > > > > locations readily to hand.
> > > > 
> > > > Would it be easier (or possible) to print the name of the eventually-
> > > > to-be-output binary? At the moment the user is left guessing which one
> > > > of 1,200 *.dtb files they just built produced each of the similar
> > > > number of warnings. If the message was instead:
> > > > 
> > > >     Warning (unit_address_vs_reg): arch/arm/boot/dts/foo.dtb: Node /soc has a ...
> > > > 
> > > > Then that would at least be something to go on.
> > > > 
> > > > In fact, given the checks are on the final tree, naming the output file
> > > >  in the messages seems fairly logical (you could even imagine doing
> > > > these checks in a separate linter tool after the fact, given the *.dtb
> > > > as input, I suppose)
> > > 
> > > Hm, possible, though a bit messy to do within dtc.  The output file is
> > 
> > s/is/isn't/  duh.
> > 
> > > currently passed into that section of the code, but I guess we could
> > > add it.
> > > 
> > > However, it seems this would more easily be fixed from the Makefile
> > > side: if you echo a (suitably abbreviated) dtc command line, then it
> > > should become obvious which dtb the errors are associated with.
> 
> Not with "make -j12" sadly. I suppose people could rerun make without
> the -j to figure out where the warning came from, but it seems rather
> suboptimal to me.

Yeah, I guess.  Given it's possible though I'm disinclined to make a
change to dtc myself.  Feel free to send a patch and I'll think about
it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Warnings do include offending filename

[Ian Campbell]

On Thu, 2017-02-02 at 16:05 +1100, David Gibson wrote:
> Feel free to send a patch and I'll think about it.

Please see below.

The diffstat is a rather large due to the need to plumb `outname`
through to all the check functions. That could perhaps be avoided by
adding it to `struct dt_info *dti` instead since that is already
available in most places, but there would still be the churn relating
to adding a parameter to `check_msg` and the various wrapper macros.
Happy to rework along those lines if you prefer it though.

Cheers,
Ian.

8<-------------

From 4fa93f32bb14f878dfe1a605b4b94b12d7e95a58 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ijc-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Date: Fri, 3 Feb 2017 08:29:39 +0000
Subject: [PATCH] Print output filename as part of warning messages

For example:
src/arm/at91-ariag25.dtb: Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name

If output is to stdout then the prefix is "<stdout>: ".

This helps to direct the developer to where to look when multiple files are
being compiled in parallel.

Quite intrusive into checks.c due to the need to plumb the output filename
throughout.

Signed-off-by: Ian Campbell <ijc-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
---
 checks.c | 143 +++++++++++++++++++++++++++++++++------------------------------
 dtc.c    |   2 +-
 dtc.h    |   2 +-
 3 files changed, 77 insertions(+), 70 deletions(-)

diff --git a/checks.c b/checks.c
index 3d18e45..f0a0e7b 100644
--- a/checks.c
+++ b/checks.c
@@ -40,7 +40,8 @@ enum checkstatus {
 
 struct check;
 
-typedef void (*check_fn)(struct check *c, struct dt_info *dti, struct node *node);
+typedef void (*check_fn)(struct check *c, struct dt_info *dti,
+			 struct node *node, const char *outname);
 
 struct check {
 	const char *name;
@@ -73,16 +74,19 @@ struct check {
 	CHECK_ENTRY(_nm, _fn, _d, false, false, __VA_ARGS__)
 
 #ifdef __GNUC__
-static inline void check_msg(struct check *c, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+static inline void check_msg(struct check *c, const char *outname,
+			     const char *fmt, ...) __attribute__((format (printf, 3, 4)));
 #endif
-static inline void check_msg(struct check *c, const char *fmt, ...)
+static inline void check_msg(struct check *c, const char *outname,
+			     const char *fmt, ...)
 {
 	va_list ap;
 	va_start(ap, fmt);
 
 	if ((c->warn && (quiet < 1))
 	    || (c->error && (quiet < 2))) {
-		fprintf(stderr, "%s (%s): ",
+		fprintf(stderr, "%s: %s (%s): ",
+			strcmp(outname, "-") ? outname : "<stdout>",
 			(c->error) ? "ERROR" : "Warning", c->name);
 		vfprintf(stderr, fmt, ap);
 		fprintf(stderr, "\n");
@@ -90,26 +94,27 @@ static inline void check_msg(struct check *c, const char *fmt, ...)
 	va_end(ap);
 }
 
-#define FAIL(c, ...) \
-	do { \
-		TRACE((c), "\t\tFAILED at %s:%d", __FILE__, __LINE__); \
-		(c)->status = FAILED; \
-		check_msg((c), __VA_ARGS__); \
+#define FAIL(c, outname, ...)						\
+	do {								\
+		TRACE((c), "\t\tFAILED at %s:%d", __FILE__, __LINE__);	\
+		(c)->status = FAILED;					\
+		check_msg((c), outname, __VA_ARGS__);			\
 	} while (0)
 
-static void check_nodes_props(struct check *c, struct dt_info *dti, struct node *node)
+static void check_nodes_props(struct check *c, struct dt_info *dti,
+			      struct node *node, const char *outname)
 {
 	struct node *child;
 
 	TRACE(c, "%s", node->fullpath);
 	if (c->fn)
-		c->fn(c, dti, node);
+		c->fn(c, dti, node, outname);
 
 	for_each_child(node, child)
-		check_nodes_props(c, dti, child);
+		check_nodes_props(c, dti, child, outname);
 }
 
-static bool run_check(struct check *c, struct dt_info *dti)
+static bool run_check(struct check *c, struct dt_info *dti, const char *outname)
 {
 	struct node *dt = dti->dt;
 	bool error = false;
@@ -124,7 +129,7 @@ static bool run_check(struct check *c, struct dt_info *dti)
 
 	for (i = 0; i < c->num_prereqs; i++) {
 		struct check *prq = c->prereq[i];
-		error = error || run_check(prq, dti);
+		error = error || run_check(prq, dti, outname);
 		if (prq->status != PASSED) {
 			c->status = PREREQ;
 			check_msg(c, "Failed prerequisite '%s'",
@@ -135,7 +140,7 @@ static bool run_check(struct check *c, struct dt_info *dti)
 	if (c->status != UNCHECKED)
 		goto out;
 
-	check_nodes_props(c, dti, dt);
+	check_nodes_props(c, dti, dt, outname);
 
 	if (c->status == UNCHECKED)
 		c->status = PASSED;
@@ -155,14 +160,14 @@ out:
 
 /* A check which always fails, for testing purposes only */
 static inline void check_always_fail(struct check *c, struct dt_info *dti,
-				     struct node *node)
+				     struct node *node, const char *outname)
 {
-	FAIL(c, "always_fail check");
+	FAIL(c, outname, "always_fail check");
 }
 CHECK(always_fail, check_always_fail, NULL);
 
 static void check_is_string(struct check *c, struct dt_info *dti,
-			    struct node *node)
+			    struct node *node, const char *outname)
 {
 	struct property *prop;
 	char *propname = c->data;
@@ -181,7 +186,7 @@ static void check_is_string(struct check *c, struct dt_info *dti,
 	ERROR(nm, check_is_string, (propname))
 
 static void check_is_cell(struct check *c, struct dt_info *dti,
-			  struct node *node)
+			  struct node *node, const char *outname)
 {
 	struct property *prop;
 	char *propname = c->data;
@@ -204,7 +209,7 @@ static void check_is_cell(struct check *c, struct dt_info *dti,
  */
 
 static void check_duplicate_node_names(struct check *c, struct dt_info *dti,
-				       struct node *node)
+				       struct node *node, const char *outname)
 {
 	struct node *child, *child2;
 
@@ -219,7 +224,7 @@ static void check_duplicate_node_names(struct check *c, struct dt_info *dti,
 ERROR(duplicate_node_names, check_duplicate_node_names, NULL);
 
 static void check_duplicate_property_names(struct check *c, struct dt_info *dti,
-					   struct node *node)
+					   struct node *node, const char *outname)
 {
 	struct property *prop, *prop2;
 
@@ -241,27 +246,27 @@ ERROR(duplicate_property_names, check_duplicate_property_names, NULL);
 #define PROPNODECHARS	LOWERCASE UPPERCASE DIGITS ",._+*#?-"
 
 static void check_node_name_chars(struct check *c, struct dt_info *dti,
-				  struct node *node)
+				  struct node *node, const char *outname)
 {
 	int n = strspn(node->name, c->data);
 
 	if (n < strlen(node->name))
-		FAIL(c, "Bad character '%c' in node %s",
+		FAIL(c, outname, "Bad character '%c' in node %s",
 		     node->name[n], node->fullpath);
 }
 ERROR(node_name_chars, check_node_name_chars, PROPNODECHARS "@");
 
 static void check_node_name_format(struct check *c, struct dt_info *dti,
-				   struct node *node)
+				   struct node *node, const char *outname)
 {
 	if (strchr(get_unitname(node), '@'))
-		FAIL(c, "Node %s has multiple '@' characters in name",
+		FAIL(c, outname, "Node %s has multiple '@' characters in name",
 		     node->fullpath);
 }
 ERROR(node_name_format, check_node_name_format, NULL, &node_name_chars);
 
 static void check_unit_address_vs_reg(struct check *c, struct dt_info *dti,
-				      struct node *node)
+				      struct node *node, const char *outname)
 {
 	const char *unitname = get_unitname(node);
 	struct property *prop = get_property(node, "reg");
@@ -274,18 +279,18 @@ static void check_unit_address_vs_reg(struct check *c, struct dt_info *dti,
 
 	if (prop) {
 		if (!unitname[0])
-			FAIL(c, "Node %s has a reg or ranges property, but no unit name",
+			FAIL(c, outname, "Node %s has a reg or ranges property, but no unit name",
 			    node->fullpath);
 	} else {
 		if (unitname[0])
-			FAIL(c, "Node %s has a unit name, but no reg property",
+			FAIL(c, outname, "Node %s has a unit name, but no reg property",
 			    node->fullpath);
 	}
 }
 WARNING(unit_address_vs_reg, check_unit_address_vs_reg, NULL);
 
 static void check_property_name_chars(struct check *c, struct dt_info *dti,
-				      struct node *node)
+				      struct node *node, const char *outname)
 {
 	struct property *prop;
 
@@ -293,7 +298,7 @@ static void check_property_name_chars(struct check *c, struct dt_info *dti,
 		int n = strspn(prop->name, c->data);
 
 		if (n < strlen(prop->name))
-			FAIL(c, "Bad character '%c' in property name \"%s\", node %s",
+			FAIL(c, outname, "Bad character '%c' in property name \"%s\", node %s",
 			     prop->name[n], prop->name, node->fullpath);
 	}
 }
@@ -308,7 +313,8 @@ ERROR(property_name_chars, check_property_name_chars, PROPNODECHARS);
 
 static void check_duplicate_label(struct check *c, struct dt_info *dti,
 				  const char *label, struct node *node,
-				  struct property *prop, struct marker *mark)
+				  struct property *prop, struct marker *mark,
+				  const char *outname)
 {
 	struct node *dt = dti->dt;
 	struct node *othernode = NULL;
@@ -327,35 +333,36 @@ static void check_duplicate_label(struct check *c, struct dt_info *dti,
 		return;
 
 	if ((othernode != node) || (otherprop != prop) || (othermark != mark))
-		FAIL(c, "Duplicate label '%s' on " DESCLABEL_FMT
+		FAIL(c, outname, "Duplicate label '%s' on " DESCLABEL_FMT
 		     " and " DESCLABEL_FMT,
 		     label, DESCLABEL_ARGS(node, prop, mark),
 		     DESCLABEL_ARGS(othernode, otherprop, othermark));
 }
 
 static void check_duplicate_label_node(struct check *c, struct dt_info *dti,
-				       struct node *node)
+				       struct node *node, const char *outname)
 {
 	struct label *l;
 	struct property *prop;
 
 	for_each_label(node->labels, l)
-		check_duplicate_label(c, dti, l->label, node, NULL, NULL);
+		check_duplicate_label(c, dti, l->label, node, NULL, NULL, outname);
 
 	for_each_property(node, prop) {
 		struct marker *m = prop->val.markers;
 
 		for_each_label(prop->labels, l)
-			check_duplicate_label(c, dti, l->label, node, prop, NULL);
+			check_duplicate_label(c, dti, l->label, node, prop, NULL, outname);
 
 		for_each_marker_of_type(m, LABEL)
-			check_duplicate_label(c, dti, m->ref, node, prop, m);
+			check_duplicate_label(c, dti, m->ref, node, prop, m, outname);
 	}
 }
 ERROR(duplicate_label, check_duplicate_label_node, NULL);
 
 static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
-				 struct node *node, const char *propname)
+				 struct node *node, const char *propname,
+				 const char *outname)
 {
 	struct node *root = dti->dt;
 	struct property *prop;
@@ -367,7 +374,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
 		return 0;
 
 	if (prop->val.len != sizeof(cell_t)) {
-		FAIL(c, "%s has bad length (%d) %s property",
+		FAIL(c, outname, "%s has bad length (%d) %s property",
 		     node->fullpath, prop->val.len, prop->name);
 		return 0;
 	}
@@ -379,7 +386,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
 			/* "Set this node's phandle equal to some
 			 * other node's phandle".  That's nonsensical
 			 * by construction. */ {
-			FAIL(c, "%s in %s is a reference to another node",
+			FAIL(c, outname, "%s in %s is a reference to another node",
 			     prop->name, node->fullpath);
 		}
 		/* But setting this node's phandle equal to its own
@@ -393,7 +400,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
 	phandle = propval_cell(prop);
 
 	if ((phandle == 0) || (phandle == -1)) {
-		FAIL(c, "%s has bad value (0x%x) in %s property",
+		FAIL(c, outname, "%s has bad value (0x%x) in %s property",
 		     node->fullpath, phandle, prop->name);
 		return 0;
 	}
@@ -402,7 +409,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
 }
 
 static void check_explicit_phandles(struct check *c, struct dt_info *dti,
-				    struct node *node)
+				    struct node *node, const char *outname)
 {
 	struct node *root = dti->dt;
 	struct node *other;
@@ -411,16 +418,16 @@ static void check_explicit_phandles(struct check *c, struct dt_info *dti,
 	/* Nothing should have assigned phandles yet */
 	assert(!node->phandle);
 
-	phandle = check_phandle_prop(c, dti, node, "phandle");
+	phandle = check_phandle_prop(c, dti, node, "phandle", outname);
 
-	linux_phandle = check_phandle_prop(c, dti, node, "linux,phandle");
+	linux_phandle = check_phandle_prop(c, dti, node, "linux,phandle", outname);
 
 	if (!phandle && !linux_phandle)
 		/* No valid phandles; nothing further to check */
 		return;
 
 	if (linux_phandle && phandle && (phandle != linux_phandle))
-		FAIL(c, "%s has mismatching 'phandle' and 'linux,phandle'"
+		FAIL(c, outname, "%s has mismatching 'phandle' and 'linux,phandle'"
 		     " properties", node->fullpath);
 
 	if (linux_phandle && !phandle)
@@ -428,7 +435,7 @@ static void check_explicit_phandles(struct check *c, struct dt_info *dti,
 
 	other = get_node_by_phandle(root, phandle);
 	if (other && (other != node)) {
-		FAIL(c, "%s has duplicated phandle 0x%x (seen before at %s)",
+		FAIL(c, outname, "%s has duplicated phandle 0x%x (seen before at %s)",
 		     node->fullpath, phandle, other->fullpath);
 		return;
 	}
@@ -438,7 +445,7 @@ static void check_explicit_phandles(struct check *c, struct dt_info *dti,
 ERROR(explicit_phandles, check_explicit_phandles, NULL);
 
 static void check_name_properties(struct check *c, struct dt_info *dti,
-				  struct node *node)
+				  struct node *node, const char *outname)
 {
 	struct property **pp, *prop = NULL;
 
@@ -453,7 +460,7 @@ static void check_name_properties(struct check *c, struct dt_info *dti,
 
 	if ((prop->val.len != node->basenamelen+1)
 	    || (memcmp(prop->val.val, node->name, node->basenamelen) != 0)) {
-		FAIL(c, "\"name\" property in %s is incorrect (\"%s\" instead"
+		FAIL(c, outname, "\"name\" property in %s is incorrect (\"%s\" instead"
 		     " of base node name)", node->fullpath, prop->val.val);
 	} else {
 		/* The name property is correct, and therefore redundant.
@@ -472,7 +479,7 @@ ERROR(name_properties, check_name_properties, NULL, &name_is_string);
  */
 
 static void fixup_phandle_references(struct check *c, struct dt_info *dti,
-				     struct node *node)
+				     struct node *node, const char *outname)
 {
 	struct node *dt = dti->dt;
 	struct property *prop;
@@ -488,7 +495,7 @@ static void fixup_phandle_references(struct check *c, struct dt_info *dti,
 			refnode = get_node_by_ref(dt, m->ref);
 			if (! refnode) {
 				if (!(dti->dtsflags & DTSF_PLUGIN))
-					FAIL(c, "Reference to non-existent node or "
+					FAIL(c, outname, "Reference to non-existent node or "
 							"label \"%s\"\n", m->ref);
 				else /* mark the entry as unresolved */
 					*((cell_t *)(prop->val.val + m->offset)) =
@@ -505,7 +512,7 @@ ERROR(phandle_references, fixup_phandle_references, NULL,
       &duplicate_node_names, &explicit_phandles);
 
 static void fixup_path_references(struct check *c, struct dt_info *dti,
-				  struct node *node)
+				  struct node *node, const char *outname)
 {
 	struct node *dt = dti->dt;
 	struct property *prop;
@@ -520,7 +527,7 @@ static void fixup_path_references(struct check *c, struct dt_info *dti,
 
 			refnode = get_node_by_ref(dt, m->ref);
 			if (!refnode) {
-				FAIL(c, "Reference to non-existent node or label \"%s\"\n",
+				FAIL(c, outname, "Reference to non-existent node or label \"%s\"\n",
 				     m->ref);
 				continue;
 			}
@@ -545,7 +552,7 @@ WARNING_IF_NOT_STRING(model_is_string, "model");
 WARNING_IF_NOT_STRING(status_is_string, "status");
 
 static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
-				  struct node *node)
+				  struct node *node, const char *outname)
 {
 	struct property *prop;
 
@@ -569,7 +576,7 @@ WARNING(addr_size_cells, fixup_addr_size_cells, NULL,
 	(((n)->size_cells == -1) ? 1 : (n)->size_cells)
 
 static void check_reg_format(struct check *c, struct dt_info *dti,
-			     struct node *node)
+			     struct node *node, const char *outname)
 {
 	struct property *prop;
 	int addr_cells, size_cells, entrylen;
@@ -579,26 +586,26 @@ static void check_reg_format(struct check *c, struct dt_info *dti,
 		return; /* No "reg", that's fine */
 
 	if (!node->parent) {
-		FAIL(c, "Root node has a \"reg\" property");
+		FAIL(c, outname, "Root node has a \"reg\" property");
 		return;
 	}
 
 	if (prop->val.len == 0)
-		FAIL(c, "\"reg\" property in %s is empty", node->fullpath);
+		FAIL(c, outname, "\"reg\" property in %s is empty", node->fullpath);
 
 	addr_cells = node_addr_cells(node->parent);
 	size_cells = node_size_cells(node->parent);
 	entrylen = (addr_cells + size_cells) * sizeof(cell_t);
 
 	if (!entrylen || (prop->val.len % entrylen) != 0)
-		FAIL(c, "\"reg\" property in %s has invalid length (%d bytes) "
+		FAIL(c, outname, "\"reg\" property in %s has invalid length (%d bytes) "
 		     "(#address-cells == %d, #size-cells == %d)",
 		     node->fullpath, prop->val.len, addr_cells, size_cells);
 }
 WARNING(reg_format, check_reg_format, NULL, &addr_size_cells);
 
 static void check_ranges_format(struct check *c, struct dt_info *dti,
-				struct node *node)
+				struct node *node, const char *outname)
 {
 	struct property *prop;
 	int c_addr_cells, p_addr_cells, c_size_cells, p_size_cells, entrylen;
@@ -608,7 +615,7 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
 		return;
 
 	if (!node->parent) {
-		FAIL(c, "Root node has a \"ranges\" property");
+		FAIL(c, outname, "Root node has a \"ranges\" property");
 		return;
 	}
 
@@ -620,17 +627,17 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
 
 	if (prop->val.len == 0) {
 		if (p_addr_cells != c_addr_cells)
-			FAIL(c, "%s has empty \"ranges\" property but its "
+			FAIL(c, outname, "%s has empty \"ranges\" property but its "
 			     "#address-cells (%d) differs from %s (%d)",
 			     node->fullpath, c_addr_cells, node->parent->fullpath,
 			     p_addr_cells);
 		if (p_size_cells != c_size_cells)
-			FAIL(c, "%s has empty \"ranges\" property but its "
+			FAIL(c, outname, "%s has empty \"ranges\" property but its "
 			     "#size-cells (%d) differs from %s (%d)",
 			     node->fullpath, c_size_cells, node->parent->fullpath,
 			     p_size_cells);
 	} else if ((prop->val.len % entrylen) != 0) {
-		FAIL(c, "\"ranges\" property in %s has invalid length (%d bytes) "
+		FAIL(c, outname, "\"ranges\" property in %s has invalid length (%d bytes) "
 		     "(parent #address-cells == %d, child #address-cells == %d, "
 		     "#size-cells == %d)", node->fullpath, prop->val.len,
 		     p_addr_cells, c_addr_cells, c_size_cells);
@@ -642,7 +649,7 @@ WARNING(ranges_format, check_ranges_format, NULL, &addr_size_cells);
  * Style checks
  */
 static void check_avoid_default_addr_size(struct check *c, struct dt_info *dti,
-					  struct node *node)
+					  struct node *node, const char *outname)
 {
 	struct property *reg, *ranges;
 
@@ -656,11 +663,11 @@ static void check_avoid_default_addr_size(struct check *c, struct dt_info *dti,
 		return;
 
 	if (node->parent->addr_cells == -1)
-		FAIL(c, "Relying on default #address-cells value for %s",
+		FAIL(c, outname, "Relying on default #address-cells value for %s",
 		     node->fullpath);
 
 	if (node->parent->size_cells == -1)
-		FAIL(c, "Relying on default #size-cells value for %s",
+		FAIL(c, outname, "Relying on default #size-cells value for %s",
 		     node->fullpath);
 }
 WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
@@ -668,7 +675,7 @@ WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
 
 static void check_obsolete_chosen_interrupt_controller(struct check *c,
 						       struct dt_info *dti,
-						       struct node *node)
+						       struct node *node, const char *outname)
 {
 	struct node *dt = dti->dt;
 	struct node *chosen;
@@ -684,7 +691,7 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
 
 	prop = get_property(chosen, "interrupt-controller");
 	if (prop)
-		FAIL(c, "/chosen has obsolete \"interrupt-controller\" "
+		FAIL(c, outname, "/chosen has obsolete \"interrupt-controller\" "
 		     "property");
 }
 WARNING(obsolete_chosen_interrupt_controller,
@@ -774,7 +781,7 @@ void parse_checks_option(bool warn, bool error, const char *arg)
 	die("Unrecognized check name \"%s\"\n", name);
 }
 
-void process_checks(bool force, struct dt_info *dti)
+void process_checks(bool force, struct dt_info *dti, const char *outname)
 {
 	int i;
 	int error = 0;
@@ -783,7 +790,7 @@ void process_checks(bool force, struct dt_info *dti)
 		struct check *c = check_table[i];
 
 		if (c->warn || c->error)
-			error = error || run_check(c, dti);
+			error = error || run_check(c, dti, outname);
 	}
 
 	if (error) {
diff --git a/dtc.c b/dtc.c
index a4edf4c..d218729 100644
--- a/dtc.c
+++ b/dtc.c
@@ -318,7 +318,7 @@ int main(int argc, char *argv[])
 		dti->boot_cpuid_phys = cmdline_boot_cpuid;
 
 	fill_fullpaths(dti->dt, "");
-	process_checks(force, dti);
+	process_checks(force, dti, outname);
 
 	/* on a plugin, generate by default */
 	if (dti->dtsflags & DTSF_PLUGIN) {
diff --git a/dtc.h b/dtc.h
index c6f125c..5eace23 100644
--- a/dtc.h
+++ b/dtc.h
@@ -263,7 +263,7 @@ void generate_local_fixups_tree(struct dt_info *dti, char *name);
 /* Checks */
 
 void parse_checks_option(bool warn, bool error, const char *arg);
-void process_checks(bool force, struct dt_info *dti);
+void process_checks(bool force, struct dt_info *dti, const char *outname);
 
 /* Flattened trees */
 
-- 
2.11.0

Re: Warnings do include offending filename

[David Gibson]

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

On Fri, Feb 03, 2017 at 07:44:06PM +0000, Ian Campbell wrote:
> On Thu, 2017-02-02 at 16:05 +1100, David Gibson wrote:
> > Feel free to send a patch and I'll think about it.
> 
> Please see below.
> 
> The diffstat is a rather large due to the need to plumb `outname`
> through to all the check functions. That could perhaps be avoided by
> adding it to `struct dt_info *dti` instead since that is already
> available in most places, but there would still be the churn relating
> to adding a parameter to `check_msg` and the various wrapper macros.
> Happy to rework along those lines if you prefer it though.

I think putting the output name into dt_info would be nicer, yes.
Make sure you allow for the case of output to stdout (which is default
for -I dtb -O dts).

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Warnings do include offending filename

[Ian Campbell]

On Fri, 2017-02-10 at 15:11 +1100, David Gibson wrote:
> On Fri, Feb 03, 2017 at 07:44:06PM +0000, Ian Campbell wrote:
> > On Thu, 2017-02-02 at 16:05 +1100, David Gibson wrote:
> > > Feel free to send a patch and I'll think about it.
> > 
> > Please see below.
> > 
> > The diffstat is a rather large due to the need to plumb `outname`
> > through to all the check functions. That could perhaps be avoided
> by
> > adding it to `struct dt_info *dti` instead since that is already
> > available in most places, but there would still be the churn
> relating
> > to adding a parameter to `check_msg` and the various wrapper
> macros.
> > Happy to rework along those lines if you prefer it though.
> 
> I think putting the output name into dt_info would be nicer, yes.

Done, see below (sorry for the delay, I've been away).

> Make sure you allow for the case of output to stdout (which is default
> for -I dtb -O dts).

I get:

$ ../dtc/dtc -I dtb -O dts src/arm/sun5i-a13-difrnce-dit4350.dtb > x.dts
<stdout>: Warning (unit_address_vs_reg): Node /chosen/framebuffer@0 has a unit name, but no reg property
<stdout>: Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
[...snip...]

Ian.

8----------

From 4434e0e8798e3770ed34e33f1e962f747f820509 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ijc-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Date: Fri, 3 Feb 2017 08:29:39 +0000
Subject: [PATCH] Print output filename as part of warning messages

For example:
src/arm/at91-ariag25.dtb: Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name

If output is to stdout then the prefix is "<stdout>: ".

This helps to direct the developer to where to look when multiple files are
being compiled in parallel.

Signed-off-by: Ian Campbell <ijc-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
---
 checks.c | 79 +++++++++++++++++++++++++++++++++-------------------------------
 dtc.c    |  2 ++
 dtc.h    |  1 +
 3 files changed, 44 insertions(+), 38 deletions(-)

diff --git a/checks.c b/checks.c
index 3d18e45..3a1dc6f 100644
--- a/checks.c
+++ b/checks.c
@@ -73,16 +73,19 @@ struct check {
 	CHECK_ENTRY(_nm, _fn, _d, false, false, __VA_ARGS__)
 
 #ifdef __GNUC__
-static inline void check_msg(struct check *c, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+static inline void check_msg(struct check *c, struct dt_info *dti,
+			     const char *fmt, ...) __attribute__((format (printf, 3, 4)));
 #endif
-static inline void check_msg(struct check *c, const char *fmt, ...)
+static inline void check_msg(struct check *c, struct dt_info *dti,
+			     const char *fmt, ...)
 {
 	va_list ap;
 	va_start(ap, fmt);
 
 	if ((c->warn && (quiet < 1))
 	    || (c->error && (quiet < 2))) {
-		fprintf(stderr, "%s (%s): ",
+		fprintf(stderr, "%s: %s (%s): ",
+			strcmp(dti->outname, "-") ? dti->outname : "<stdout>",
 			(c->error) ? "ERROR" : "Warning", c->name);
 		vfprintf(stderr, fmt, ap);
 		fprintf(stderr, "\n");
@@ -90,11 +93,11 @@ static inline void check_msg(struct check *c, const char *fmt, ...)
 	va_end(ap);
 }
 
-#define FAIL(c, ...) \
-	do { \
-		TRACE((c), "\t\tFAILED at %s:%d", __FILE__, __LINE__); \
-		(c)->status = FAILED; \
-		check_msg((c), __VA_ARGS__); \
+#define FAIL(c, dti, ...)						\
+	do {								\
+		TRACE((c), "\t\tFAILED at %s:%d", __FILE__, __LINE__);	\
+		(c)->status = FAILED;					\
+		check_msg((c), dti, __VA_ARGS__);			\
 	} while (0)
 
 static void check_nodes_props(struct check *c, struct dt_info *dti, struct node *node)
@@ -127,7 +130,7 @@ static bool run_check(struct check *c, struct dt_info *dti)
 		error = error || run_check(prq, dti);
 		if (prq->status != PASSED) {
 			c->status = PREREQ;
-			check_msg(c, "Failed prerequisite '%s'",
+			check_msg(c, dti, "Failed prerequisite '%s'",
 				  c->prereq[i]->name);
 		}
 	}
@@ -157,7 +160,7 @@ out:
 static inline void check_always_fail(struct check *c, struct dt_info *dti,
 				     struct node *node)
 {
-	FAIL(c, "always_fail check");
+	FAIL(c, dti, "always_fail check");
 }
 CHECK(always_fail, check_always_fail, NULL);
 
@@ -172,7 +175,7 @@ static void check_is_string(struct check *c, struct dt_info *dti,
 		return; /* Not present, assumed ok */
 
 	if (!data_is_one_string(prop->val))
-		FAIL(c, "\"%s\" property in %s is not a string",
+		FAIL(c, dti, "\"%s\" property in %s is not a string",
 		     propname, node->fullpath);
 }
 #define WARNING_IF_NOT_STRING(nm, propname) \
@@ -191,7 +194,7 @@ static void check_is_cell(struct check *c, struct dt_info *dti,
 		return; /* Not present, assumed ok */
 
 	if (prop->val.len != sizeof(cell_t))
-		FAIL(c, "\"%s\" property in %s is not a single cell",
+		FAIL(c, dti, "\"%s\" property in %s is not a single cell",
 		     propname, node->fullpath);
 }
 #define WARNING_IF_NOT_CELL(nm, propname) \
@@ -213,7 +216,7 @@ static void check_duplicate_node_names(struct check *c, struct dt_info *dti,
 		     child2;
 		     child2 = child2->next_sibling)
 			if (streq(child->name, child2->name))
-				FAIL(c, "Duplicate node name %s",
+				FAIL(c, dti, "Duplicate node name %s",
 				     child->fullpath);
 }
 ERROR(duplicate_node_names, check_duplicate_node_names, NULL);
@@ -228,7 +231,7 @@ static void check_duplicate_property_names(struct check *c, struct dt_info *dti,
 			if (prop2->deleted)
 				continue;
 			if (streq(prop->name, prop2->name))
-				FAIL(c, "Duplicate property name %s in %s",
+				FAIL(c, dti, "Duplicate property name %s in %s",
 				     prop->name, node->fullpath);
 		}
 	}
@@ -246,7 +249,7 @@ static void check_node_name_chars(struct check *c, struct dt_info *dti,
 	int n = strspn(node->name, c->data);
 
 	if (n < strlen(node->name))
-		FAIL(c, "Bad character '%c' in node %s",
+		FAIL(c, dti, "Bad character '%c' in node %s",
 		     node->name[n], node->fullpath);
 }
 ERROR(node_name_chars, check_node_name_chars, PROPNODECHARS "@");
@@ -255,7 +258,7 @@ static void check_node_name_format(struct check *c, struct dt_info *dti,
 				   struct node *node)
 {
 	if (strchr(get_unitname(node), '@'))
-		FAIL(c, "Node %s has multiple '@' characters in name",
+		FAIL(c, dti, "Node %s has multiple '@' characters in name",
 		     node->fullpath);
 }
 ERROR(node_name_format, check_node_name_format, NULL, &node_name_chars);
@@ -274,11 +277,11 @@ static void check_unit_address_vs_reg(struct check *c, struct dt_info *dti,
 
 	if (prop) {
 		if (!unitname[0])
-			FAIL(c, "Node %s has a reg or ranges property, but no unit name",
+			FAIL(c, dti, "Node %s has a reg or ranges property, but no unit name",
 			    node->fullpath);
 	} else {
 		if (unitname[0])
-			FAIL(c, "Node %s has a unit name, but no reg property",
+			FAIL(c, dti, "Node %s has a unit name, but no reg property",
 			    node->fullpath);
 	}
 }
@@ -293,7 +296,7 @@ static void check_property_name_chars(struct check *c, struct dt_info *dti,
 		int n = strspn(prop->name, c->data);
 
 		if (n < strlen(prop->name))
-			FAIL(c, "Bad character '%c' in property name \"%s\", node %s",
+			FAIL(c, dti, "Bad character '%c' in property name \"%s\", node %s",
 			     prop->name[n], prop->name, node->fullpath);
 	}
 }
@@ -327,7 +330,7 @@ static void check_duplicate_label(struct check *c, struct dt_info *dti,
 		return;
 
 	if ((othernode != node) || (otherprop != prop) || (othermark != mark))
-		FAIL(c, "Duplicate label '%s' on " DESCLABEL_FMT
+		FAIL(c, dti, "Duplicate label '%s' on " DESCLABEL_FMT
 		     " and " DESCLABEL_FMT,
 		     label, DESCLABEL_ARGS(node, prop, mark),
 		     DESCLABEL_ARGS(othernode, otherprop, othermark));
@@ -367,7 +370,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
 		return 0;
 
 	if (prop->val.len != sizeof(cell_t)) {
-		FAIL(c, "%s has bad length (%d) %s property",
+		FAIL(c, dti, "%s has bad length (%d) %s property",
 		     node->fullpath, prop->val.len, prop->name);
 		return 0;
 	}
@@ -379,7 +382,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
 			/* "Set this node's phandle equal to some
 			 * other node's phandle".  That's nonsensical
 			 * by construction. */ {
-			FAIL(c, "%s in %s is a reference to another node",
+			FAIL(c, dti, "%s in %s is a reference to another node",
 			     prop->name, node->fullpath);
 		}
 		/* But setting this node's phandle equal to its own
@@ -393,7 +396,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
 	phandle = propval_cell(prop);
 
 	if ((phandle == 0) || (phandle == -1)) {
-		FAIL(c, "%s has bad value (0x%x) in %s property",
+		FAIL(c, dti, "%s has bad value (0x%x) in %s property",
 		     node->fullpath, phandle, prop->name);
 		return 0;
 	}
@@ -420,7 +423,7 @@ static void check_explicit_phandles(struct check *c, struct dt_info *dti,
 		return;
 
 	if (linux_phandle && phandle && (phandle != linux_phandle))
-		FAIL(c, "%s has mismatching 'phandle' and 'linux,phandle'"
+		FAIL(c, dti, "%s has mismatching 'phandle' and 'linux,phandle'"
 		     " properties", node->fullpath);
 
 	if (linux_phandle && !phandle)
@@ -428,7 +431,7 @@ static void check_explicit_phandles(struct check *c, struct dt_info *dti,
 
 	other = get_node_by_phandle(root, phandle);
 	if (other && (other != node)) {
-		FAIL(c, "%s has duplicated phandle 0x%x (seen before at %s)",
+		FAIL(c, dti, "%s has duplicated phandle 0x%x (seen before at %s)",
 		     node->fullpath, phandle, other->fullpath);
 		return;
 	}
@@ -453,7 +456,7 @@ static void check_name_properties(struct check *c, struct dt_info *dti,
 
 	if ((prop->val.len != node->basenamelen+1)
 	    || (memcmp(prop->val.val, node->name, node->basenamelen) != 0)) {
-		FAIL(c, "\"name\" property in %s is incorrect (\"%s\" instead"
+		FAIL(c, dti, "\"name\" property in %s is incorrect (\"%s\" instead"
 		     " of base node name)", node->fullpath, prop->val.val);
 	} else {
 		/* The name property is correct, and therefore redundant.
@@ -488,7 +491,7 @@ static void fixup_phandle_references(struct check *c, struct dt_info *dti,
 			refnode = get_node_by_ref(dt, m->ref);
 			if (! refnode) {
 				if (!(dti->dtsflags & DTSF_PLUGIN))
-					FAIL(c, "Reference to non-existent node or "
+					FAIL(c, dti, "Reference to non-existent node or "
 							"label \"%s\"\n", m->ref);
 				else /* mark the entry as unresolved */
 					*((cell_t *)(prop->val.val + m->offset)) =
@@ -520,7 +523,7 @@ static void fixup_path_references(struct check *c, struct dt_info *dti,
 
 			refnode = get_node_by_ref(dt, m->ref);
 			if (!refnode) {
-				FAIL(c, "Reference to non-existent node or label \"%s\"\n",
+				FAIL(c, dti, "Reference to non-existent node or label \"%s\"\n",
 				     m->ref);
 				continue;
 			}
@@ -579,19 +582,19 @@ static void check_reg_format(struct check *c, struct dt_info *dti,
 		return; /* No "reg", that's fine */
 
 	if (!node->parent) {
-		FAIL(c, "Root node has a \"reg\" property");
+		FAIL(c, dti, "Root node has a \"reg\" property");
 		return;
 	}
 
 	if (prop->val.len == 0)
-		FAIL(c, "\"reg\" property in %s is empty", node->fullpath);
+		FAIL(c, dti, "\"reg\" property in %s is empty", node->fullpath);
 
 	addr_cells = node_addr_cells(node->parent);
 	size_cells = node_size_cells(node->parent);
 	entrylen = (addr_cells + size_cells) * sizeof(cell_t);
 
 	if (!entrylen || (prop->val.len % entrylen) != 0)
-		FAIL(c, "\"reg\" property in %s has invalid length (%d bytes) "
+		FAIL(c, dti, "\"reg\" property in %s has invalid length (%d bytes) "
 		     "(#address-cells == %d, #size-cells == %d)",
 		     node->fullpath, prop->val.len, addr_cells, size_cells);
 }
@@ -608,7 +611,7 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
 		return;
 
 	if (!node->parent) {
-		FAIL(c, "Root node has a \"ranges\" property");
+		FAIL(c, dti, "Root node has a \"ranges\" property");
 		return;
 	}
 
@@ -620,17 +623,17 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
 
 	if (prop->val.len == 0) {
 		if (p_addr_cells != c_addr_cells)
-			FAIL(c, "%s has empty \"ranges\" property but its "
+			FAIL(c, dti, "%s has empty \"ranges\" property but its "
 			     "#address-cells (%d) differs from %s (%d)",
 			     node->fullpath, c_addr_cells, node->parent->fullpath,
 			     p_addr_cells);
 		if (p_size_cells != c_size_cells)
-			FAIL(c, "%s has empty \"ranges\" property but its "
+			FAIL(c, dti, "%s has empty \"ranges\" property but its "
 			     "#size-cells (%d) differs from %s (%d)",
 			     node->fullpath, c_size_cells, node->parent->fullpath,
 			     p_size_cells);
 	} else if ((prop->val.len % entrylen) != 0) {
-		FAIL(c, "\"ranges\" property in %s has invalid length (%d bytes) "
+		FAIL(c, dti, "\"ranges\" property in %s has invalid length (%d bytes) "
 		     "(parent #address-cells == %d, child #address-cells == %d, "
 		     "#size-cells == %d)", node->fullpath, prop->val.len,
 		     p_addr_cells, c_addr_cells, c_size_cells);
@@ -656,11 +659,11 @@ static void check_avoid_default_addr_size(struct check *c, struct dt_info *dti,
 		return;
 
 	if (node->parent->addr_cells == -1)
-		FAIL(c, "Relying on default #address-cells value for %s",
+		FAIL(c, dti, "Relying on default #address-cells value for %s",
 		     node->fullpath);
 
 	if (node->parent->size_cells == -1)
-		FAIL(c, "Relying on default #size-cells value for %s",
+		FAIL(c, dti, "Relying on default #size-cells value for %s",
 		     node->fullpath);
 }
 WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
@@ -684,7 +687,7 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
 
 	prop = get_property(chosen, "interrupt-controller");
 	if (prop)
-		FAIL(c, "/chosen has obsolete \"interrupt-controller\" "
+		FAIL(c, dti, "/chosen has obsolete \"interrupt-controller\" "
 		     "property");
 }
 WARNING(obsolete_chosen_interrupt_controller,
diff --git a/dtc.c b/dtc.c
index a4edf4c..9c30c33 100644
--- a/dtc.c
+++ b/dtc.c
@@ -309,6 +309,8 @@ int main(int argc, char *argv[])
 	else
 		die("Unknown input format \"%s\"\n", inform);
 
+	dti->outname = outname;
+
 	if (depfile) {
 		fputc('\n', depfile);
 		fclose(depfile);
diff --git a/dtc.h b/dtc.h
index c6f125c..1ac2a1e 100644
--- a/dtc.h
+++ b/dtc.h
@@ -246,6 +246,7 @@ struct dt_info {
 	struct reserve_info *reservelist;
 	uint32_t boot_cpuid_phys;
 	struct node *dt;		/* the device tree */
+	const char *outname;		/* filename being written to, "-" for stdout */
 };
 
 /* DTS version flags definitions */
-- 
2.11.0

Re: Warnings do include offending filename

[David Gibson]

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

On Sun, Feb 19, 2017 at 04:00:43PM +0000, Ian Campbell wrote:
> On Fri, 2017-02-10 at 15:11 +1100, David Gibson wrote:
> > On Fri, Feb 03, 2017 at 07:44:06PM +0000, Ian Campbell wrote:
> > > On Thu, 2017-02-02 at 16:05 +1100, David Gibson wrote:
> > > > Feel free to send a patch and I'll think about it.
> > > 
> > > Please see below.
> > > 
> > > The diffstat is a rather large due to the need to plumb `outname`
> > > through to all the check functions. That could perhaps be avoided
> > by
> > > adding it to `struct dt_info *dti` instead since that is already
> > > available in most places, but there would still be the churn
> > relating
> > > to adding a parameter to `check_msg` and the various wrapper
> > macros.
> > > Happy to rework along those lines if you prefer it though.
> > 
> > I think putting the output name into dt_info would be nicer, yes.
> 
> Done, see below (sorry for the delay, I've been away).

Sorry for my delay, I've been busy.

> > Make sure you allow for the case of output to stdout (which is default
> > for -I dtb -O dts).
> 
> I get:
> 
> $ ../dtc/dtc -I dtb -O dts src/arm/sun5i-a13-difrnce-dit4350.dtb > x.dts
> <stdout>: Warning (unit_address_vs_reg): Node /chosen/framebuffer@0 has a unit name, but no reg property
> <stdout>: Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
> [...snip...]

Ok looks good.  Except that since you wrote it I've merged some other
patches adding more FAIL() calls, so this now causes compile errors.

Can you please rebase onto current master, fix up for the new calls
and resend.

> 
> Ian.
> 
> 8----------
> 
> >From 4434e0e8798e3770ed34e33f1e962f747f820509 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ijc-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
> Date: Fri, 3 Feb 2017 08:29:39 +0000
> Subject: [PATCH] Print output filename as part of warning messages
> 
> For example:
> src/arm/at91-ariag25.dtb: Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
> 
> If output is to stdout then the prefix is "<stdout>: ".
> 
> This helps to direct the developer to where to look when multiple files are
> being compiled in parallel.
> 
> Signed-off-by: Ian Campbell <ijc-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
> ---
>  checks.c | 79 +++++++++++++++++++++++++++++++++-------------------------------
>  dtc.c    |  2 ++
>  dtc.h    |  1 +
>  3 files changed, 44 insertions(+), 38 deletions(-)
> 
> diff --git a/checks.c b/checks.c
> index 3d18e45..3a1dc6f 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -73,16 +73,19 @@ struct check {
>  	CHECK_ENTRY(_nm, _fn, _d, false, false, __VA_ARGS__)
>  
>  #ifdef __GNUC__
> -static inline void check_msg(struct check *c, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
> +static inline void check_msg(struct check *c, struct dt_info *dti,
> +			     const char *fmt, ...) __attribute__((format (printf, 3, 4)));
>  #endif
> -static inline void check_msg(struct check *c, const char *fmt, ...)
> +static inline void check_msg(struct check *c, struct dt_info *dti,
> +			     const char *fmt, ...)
>  {
>  	va_list ap;
>  	va_start(ap, fmt);
>  
>  	if ((c->warn && (quiet < 1))
>  	    || (c->error && (quiet < 2))) {
> -		fprintf(stderr, "%s (%s): ",
> +		fprintf(stderr, "%s: %s (%s): ",
> +			strcmp(dti->outname, "-") ? dti->outname : "<stdout>",
>  			(c->error) ? "ERROR" : "Warning", c->name);
>  		vfprintf(stderr, fmt, ap);
>  		fprintf(stderr, "\n");
> @@ -90,11 +93,11 @@ static inline void check_msg(struct check *c, const char *fmt, ...)
>  	va_end(ap);
>  }
>  
> -#define FAIL(c, ...) \
> -	do { \
> -		TRACE((c), "\t\tFAILED at %s:%d", __FILE__, __LINE__); \
> -		(c)->status = FAILED; \
> -		check_msg((c), __VA_ARGS__); \
> +#define FAIL(c, dti, ...)						\
> +	do {								\
> +		TRACE((c), "\t\tFAILED at %s:%d", __FILE__, __LINE__);	\
> +		(c)->status = FAILED;					\
> +		check_msg((c), dti, __VA_ARGS__);			\
>  	} while (0)
>  
>  static void check_nodes_props(struct check *c, struct dt_info *dti, struct node *node)
> @@ -127,7 +130,7 @@ static bool run_check(struct check *c, struct dt_info *dti)
>  		error = error || run_check(prq, dti);
>  		if (prq->status != PASSED) {
>  			c->status = PREREQ;
> -			check_msg(c, "Failed prerequisite '%s'",
> +			check_msg(c, dti, "Failed prerequisite '%s'",
>  				  c->prereq[i]->name);
>  		}
>  	}
> @@ -157,7 +160,7 @@ out:
>  static inline void check_always_fail(struct check *c, struct dt_info *dti,
>  				     struct node *node)
>  {
> -	FAIL(c, "always_fail check");
> +	FAIL(c, dti, "always_fail check");
>  }
>  CHECK(always_fail, check_always_fail, NULL);
>  
> @@ -172,7 +175,7 @@ static void check_is_string(struct check *c, struct dt_info *dti,
>  		return; /* Not present, assumed ok */
>  
>  	if (!data_is_one_string(prop->val))
> -		FAIL(c, "\"%s\" property in %s is not a string",
> +		FAIL(c, dti, "\"%s\" property in %s is not a string",
>  		     propname, node->fullpath);
>  }
>  #define WARNING_IF_NOT_STRING(nm, propname) \
> @@ -191,7 +194,7 @@ static void check_is_cell(struct check *c, struct dt_info *dti,
>  		return; /* Not present, assumed ok */
>  
>  	if (prop->val.len != sizeof(cell_t))
> -		FAIL(c, "\"%s\" property in %s is not a single cell",
> +		FAIL(c, dti, "\"%s\" property in %s is not a single cell",
>  		     propname, node->fullpath);
>  }
>  #define WARNING_IF_NOT_CELL(nm, propname) \
> @@ -213,7 +216,7 @@ static void check_duplicate_node_names(struct check *c, struct dt_info *dti,
>  		     child2;
>  		     child2 = child2->next_sibling)
>  			if (streq(child->name, child2->name))
> -				FAIL(c, "Duplicate node name %s",
> +				FAIL(c, dti, "Duplicate node name %s",
>  				     child->fullpath);
>  }
>  ERROR(duplicate_node_names, check_duplicate_node_names, NULL);
> @@ -228,7 +231,7 @@ static void check_duplicate_property_names(struct check *c, struct dt_info *dti,
>  			if (prop2->deleted)
>  				continue;
>  			if (streq(prop->name, prop2->name))
> -				FAIL(c, "Duplicate property name %s in %s",
> +				FAIL(c, dti, "Duplicate property name %s in %s",
>  				     prop->name, node->fullpath);
>  		}
>  	}
> @@ -246,7 +249,7 @@ static void check_node_name_chars(struct check *c, struct dt_info *dti,
>  	int n = strspn(node->name, c->data);
>  
>  	if (n < strlen(node->name))
> -		FAIL(c, "Bad character '%c' in node %s",
> +		FAIL(c, dti, "Bad character '%c' in node %s",
>  		     node->name[n], node->fullpath);
>  }
>  ERROR(node_name_chars, check_node_name_chars, PROPNODECHARS "@");
> @@ -255,7 +258,7 @@ static void check_node_name_format(struct check *c, struct dt_info *dti,
>  				   struct node *node)
>  {
>  	if (strchr(get_unitname(node), '@'))
> -		FAIL(c, "Node %s has multiple '@' characters in name",
> +		FAIL(c, dti, "Node %s has multiple '@' characters in name",
>  		     node->fullpath);
>  }
>  ERROR(node_name_format, check_node_name_format, NULL, &node_name_chars);
> @@ -274,11 +277,11 @@ static void check_unit_address_vs_reg(struct check *c, struct dt_info *dti,
>  
>  	if (prop) {
>  		if (!unitname[0])
> -			FAIL(c, "Node %s has a reg or ranges property, but no unit name",
> +			FAIL(c, dti, "Node %s has a reg or ranges property, but no unit name",
>  			    node->fullpath);
>  	} else {
>  		if (unitname[0])
> -			FAIL(c, "Node %s has a unit name, but no reg property",
> +			FAIL(c, dti, "Node %s has a unit name, but no reg property",
>  			    node->fullpath);
>  	}
>  }
> @@ -293,7 +296,7 @@ static void check_property_name_chars(struct check *c, struct dt_info *dti,
>  		int n = strspn(prop->name, c->data);
>  
>  		if (n < strlen(prop->name))
> -			FAIL(c, "Bad character '%c' in property name \"%s\", node %s",
> +			FAIL(c, dti, "Bad character '%c' in property name \"%s\", node %s",
>  			     prop->name[n], prop->name, node->fullpath);
>  	}
>  }
> @@ -327,7 +330,7 @@ static void check_duplicate_label(struct check *c, struct dt_info *dti,
>  		return;
>  
>  	if ((othernode != node) || (otherprop != prop) || (othermark != mark))
> -		FAIL(c, "Duplicate label '%s' on " DESCLABEL_FMT
> +		FAIL(c, dti, "Duplicate label '%s' on " DESCLABEL_FMT
>  		     " and " DESCLABEL_FMT,
>  		     label, DESCLABEL_ARGS(node, prop, mark),
>  		     DESCLABEL_ARGS(othernode, otherprop, othermark));
> @@ -367,7 +370,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
>  		return 0;
>  
>  	if (prop->val.len != sizeof(cell_t)) {
> -		FAIL(c, "%s has bad length (%d) %s property",
> +		FAIL(c, dti, "%s has bad length (%d) %s property",
>  		     node->fullpath, prop->val.len, prop->name);
>  		return 0;
>  	}
> @@ -379,7 +382,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
>  			/* "Set this node's phandle equal to some
>  			 * other node's phandle".  That's nonsensical
>  			 * by construction. */ {
> -			FAIL(c, "%s in %s is a reference to another node",
> +			FAIL(c, dti, "%s in %s is a reference to another node",
>  			     prop->name, node->fullpath);
>  		}
>  		/* But setting this node's phandle equal to its own
> @@ -393,7 +396,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
>  	phandle = propval_cell(prop);
>  
>  	if ((phandle == 0) || (phandle == -1)) {
> -		FAIL(c, "%s has bad value (0x%x) in %s property",
> +		FAIL(c, dti, "%s has bad value (0x%x) in %s property",
>  		     node->fullpath, phandle, prop->name);
>  		return 0;
>  	}
> @@ -420,7 +423,7 @@ static void check_explicit_phandles(struct check *c, struct dt_info *dti,
>  		return;
>  
>  	if (linux_phandle && phandle && (phandle != linux_phandle))
> -		FAIL(c, "%s has mismatching 'phandle' and 'linux,phandle'"
> +		FAIL(c, dti, "%s has mismatching 'phandle' and 'linux,phandle'"
>  		     " properties", node->fullpath);
>  
>  	if (linux_phandle && !phandle)
> @@ -428,7 +431,7 @@ static void check_explicit_phandles(struct check *c, struct dt_info *dti,
>  
>  	other = get_node_by_phandle(root, phandle);
>  	if (other && (other != node)) {
> -		FAIL(c, "%s has duplicated phandle 0x%x (seen before at %s)",
> +		FAIL(c, dti, "%s has duplicated phandle 0x%x (seen before at %s)",
>  		     node->fullpath, phandle, other->fullpath);
>  		return;
>  	}
> @@ -453,7 +456,7 @@ static void check_name_properties(struct check *c, struct dt_info *dti,
>  
>  	if ((prop->val.len != node->basenamelen+1)
>  	    || (memcmp(prop->val.val, node->name, node->basenamelen) != 0)) {
> -		FAIL(c, "\"name\" property in %s is incorrect (\"%s\" instead"
> +		FAIL(c, dti, "\"name\" property in %s is incorrect (\"%s\" instead"
>  		     " of base node name)", node->fullpath, prop->val.val);
>  	} else {
>  		/* The name property is correct, and therefore redundant.
> @@ -488,7 +491,7 @@ static void fixup_phandle_references(struct check *c, struct dt_info *dti,
>  			refnode = get_node_by_ref(dt, m->ref);
>  			if (! refnode) {
>  				if (!(dti->dtsflags & DTSF_PLUGIN))
> -					FAIL(c, "Reference to non-existent node or "
> +					FAIL(c, dti, "Reference to non-existent node or "
>  							"label \"%s\"\n", m->ref);
>  				else /* mark the entry as unresolved */
>  					*((cell_t *)(prop->val.val + m->offset)) =
> @@ -520,7 +523,7 @@ static void fixup_path_references(struct check *c, struct dt_info *dti,
>  
>  			refnode = get_node_by_ref(dt, m->ref);
>  			if (!refnode) {
> -				FAIL(c, "Reference to non-existent node or label \"%s\"\n",
> +				FAIL(c, dti, "Reference to non-existent node or label \"%s\"\n",
>  				     m->ref);
>  				continue;
>  			}
> @@ -579,19 +582,19 @@ static void check_reg_format(struct check *c, struct dt_info *dti,
>  		return; /* No "reg", that's fine */
>  
>  	if (!node->parent) {
> -		FAIL(c, "Root node has a \"reg\" property");
> +		FAIL(c, dti, "Root node has a \"reg\" property");
>  		return;
>  	}
>  
>  	if (prop->val.len == 0)
> -		FAIL(c, "\"reg\" property in %s is empty", node->fullpath);
> +		FAIL(c, dti, "\"reg\" property in %s is empty", node->fullpath);
>  
>  	addr_cells = node_addr_cells(node->parent);
>  	size_cells = node_size_cells(node->parent);
>  	entrylen = (addr_cells + size_cells) * sizeof(cell_t);
>  
>  	if (!entrylen || (prop->val.len % entrylen) != 0)
> -		FAIL(c, "\"reg\" property in %s has invalid length (%d bytes) "
> +		FAIL(c, dti, "\"reg\" property in %s has invalid length (%d bytes) "
>  		     "(#address-cells == %d, #size-cells == %d)",
>  		     node->fullpath, prop->val.len, addr_cells, size_cells);
>  }
> @@ -608,7 +611,7 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
>  		return;
>  
>  	if (!node->parent) {
> -		FAIL(c, "Root node has a \"ranges\" property");
> +		FAIL(c, dti, "Root node has a \"ranges\" property");
>  		return;
>  	}
>  
> @@ -620,17 +623,17 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
>  
>  	if (prop->val.len == 0) {
>  		if (p_addr_cells != c_addr_cells)
> -			FAIL(c, "%s has empty \"ranges\" property but its "
> +			FAIL(c, dti, "%s has empty \"ranges\" property but its "
>  			     "#address-cells (%d) differs from %s (%d)",
>  			     node->fullpath, c_addr_cells, node->parent->fullpath,
>  			     p_addr_cells);
>  		if (p_size_cells != c_size_cells)
> -			FAIL(c, "%s has empty \"ranges\" property but its "
> +			FAIL(c, dti, "%s has empty \"ranges\" property but its "
>  			     "#size-cells (%d) differs from %s (%d)",
>  			     node->fullpath, c_size_cells, node->parent->fullpath,
>  			     p_size_cells);
>  	} else if ((prop->val.len % entrylen) != 0) {
> -		FAIL(c, "\"ranges\" property in %s has invalid length (%d bytes) "
> +		FAIL(c, dti, "\"ranges\" property in %s has invalid length (%d bytes) "
>  		     "(parent #address-cells == %d, child #address-cells == %d, "
>  		     "#size-cells == %d)", node->fullpath, prop->val.len,
>  		     p_addr_cells, c_addr_cells, c_size_cells);
> @@ -656,11 +659,11 @@ static void check_avoid_default_addr_size(struct check *c, struct dt_info *dti,
>  		return;
>  
>  	if (node->parent->addr_cells == -1)
> -		FAIL(c, "Relying on default #address-cells value for %s",
> +		FAIL(c, dti, "Relying on default #address-cells value for %s",
>  		     node->fullpath);
>  
>  	if (node->parent->size_cells == -1)
> -		FAIL(c, "Relying on default #size-cells value for %s",
> +		FAIL(c, dti, "Relying on default #size-cells value for %s",
>  		     node->fullpath);
>  }
>  WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
> @@ -684,7 +687,7 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
>  
>  	prop = get_property(chosen, "interrupt-controller");
>  	if (prop)
> -		FAIL(c, "/chosen has obsolete \"interrupt-controller\" "
> +		FAIL(c, dti, "/chosen has obsolete \"interrupt-controller\" "
>  		     "property");
>  }
>  WARNING(obsolete_chosen_interrupt_controller,
> diff --git a/dtc.c b/dtc.c
> index a4edf4c..9c30c33 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -309,6 +309,8 @@ int main(int argc, char *argv[])
>  	else
>  		die("Unknown input format \"%s\"\n", inform);
>  
> +	dti->outname = outname;
> +
>  	if (depfile) {
>  		fputc('\n', depfile);
>  		fclose(depfile);
> diff --git a/dtc.h b/dtc.h
> index c6f125c..1ac2a1e 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -246,6 +246,7 @@ struct dt_info {
>  	struct reserve_info *reservelist;
>  	uint32_t boot_cpuid_phys;
>  	struct node *dt;		/* the device tree */
> +	const char *outname;		/* filename being written to, "-" for stdout */
>  };
>  
>  /* DTS version flags definitions */

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Warnings do include offending filename

[Ian Campbell]

On Thu, 2017-02-23 at 14:42 +1100, David Gibson wrote:
> Ok looks good.  Except that since you wrote it I've merged some other
> patches adding more FAIL() calls, so this now causes compile errors.
> 
> Can you please rebase onto current master, fix up for the new calls
> and resend.

Done.

8----------

From 011d25c56ba77f1050c4c9d1f15b531ce0a4ab9d Mon Sep 17 00:00:00 2001
From: Ian Campbell <ijc-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Date: Fri, 3 Feb 2017 08:29:39 +0000
Subject: [PATCH] Print output filename as part of warning messages

For example:
src/arm/at91-ariag25.dtb: Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name

If output is to stdout then the prefix is "<stdout>: ".

This helps to direct the developer to where to look when multiple files are
being compiled in parallel.

Signed-off-by: Ian Campbell <ijc-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
---
 checks.c | 83 +++++++++++++++++++++++++++++++++-------------------------------
 dtc.c    |  2 ++
 dtc.h    |  1 +
 3 files changed, 46 insertions(+), 40 deletions(-)

diff --git a/checks.c b/checks.c
index cce0cca..0e8b978 100644
--- a/checks.c
+++ b/checks.c
@@ -73,16 +73,19 @@ struct check {
 	CHECK_ENTRY(_nm, _fn, _d, false, false, __VA_ARGS__)
 
 #ifdef __GNUC__
-static inline void check_msg(struct check *c, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+static inline void check_msg(struct check *c, struct dt_info *dti,
+			     const char *fmt, ...) __attribute__((format (printf, 3, 4)));
 #endif
-static inline void check_msg(struct check *c, const char *fmt, ...)
+static inline void check_msg(struct check *c, struct dt_info *dti,
+			     const char *fmt, ...)
 {
 	va_list ap;
 	va_start(ap, fmt);
 
 	if ((c->warn && (quiet < 1))
 	    || (c->error && (quiet < 2))) {
-		fprintf(stderr, "%s (%s): ",
+		fprintf(stderr, "%s: %s (%s): ",
+			strcmp(dti->outname, "-") ? dti->outname : "<stdout>",
 			(c->error) ? "ERROR" : "Warning", c->name);
 		vfprintf(stderr, fmt, ap);
 		fprintf(stderr, "\n");
@@ -90,11 +93,11 @@ static inline void check_msg(struct check *c, const char *fmt, ...)
 	va_end(ap);
 }
 
-#define FAIL(c, ...) \
-	do { \
-		TRACE((c), "\t\tFAILED at %s:%d", __FILE__, __LINE__); \
-		(c)->status = FAILED; \
-		check_msg((c), __VA_ARGS__); \
+#define FAIL(c, dti, ...)						\
+	do {								\
+		TRACE((c), "\t\tFAILED at %s:%d", __FILE__, __LINE__);	\
+		(c)->status = FAILED;					\
+		check_msg((c), dti, __VA_ARGS__);			\
 	} while (0)
 
 static void check_nodes_props(struct check *c, struct dt_info *dti, struct node *node)
@@ -127,7 +130,7 @@ static bool run_check(struct check *c, struct dt_info *dti)
 		error = error || run_check(prq, dti);
 		if (prq->status != PASSED) {
 			c->status = PREREQ;
-			check_msg(c, "Failed prerequisite '%s'",
+			check_msg(c, dti, "Failed prerequisite '%s'",
 				  c->prereq[i]->name);
 		}
 	}
@@ -157,7 +160,7 @@ out:
 static inline void check_always_fail(struct check *c, struct dt_info *dti,
 				     struct node *node)
 {
-	FAIL(c, "always_fail check");
+	FAIL(c, dti, "always_fail check");
 }
 CHECK(always_fail, check_always_fail, NULL);
 
@@ -172,7 +175,7 @@ static void check_is_string(struct check *c, struct dt_info *dti,
 		return; /* Not present, assumed ok */
 
 	if (!data_is_one_string(prop->val))
-		FAIL(c, "\"%s\" property in %s is not a string",
+		FAIL(c, dti, "\"%s\" property in %s is not a string",
 		     propname, node->fullpath);
 }
 #define WARNING_IF_NOT_STRING(nm, propname) \
@@ -191,7 +194,7 @@ static void check_is_cell(struct check *c, struct dt_info *dti,
 		return; /* Not present, assumed ok */
 
 	if (prop->val.len != sizeof(cell_t))
-		FAIL(c, "\"%s\" property in %s is not a single cell",
+		FAIL(c, dti, "\"%s\" property in %s is not a single cell",
 		     propname, node->fullpath);
 }
 #define WARNING_IF_NOT_CELL(nm, propname) \
@@ -213,7 +216,7 @@ static void check_duplicate_node_names(struct check *c, struct dt_info *dti,
 		     child2;
 		     child2 = child2->next_sibling)
 			if (streq(child->name, child2->name))
-				FAIL(c, "Duplicate node name %s",
+				FAIL(c, dti, "Duplicate node name %s",
 				     child->fullpath);
 }
 ERROR(duplicate_node_names, check_duplicate_node_names, NULL);
@@ -228,7 +231,7 @@ static void check_duplicate_property_names(struct check *c, struct dt_info *dti,
 			if (prop2->deleted)
 				continue;
 			if (streq(prop->name, prop2->name))
-				FAIL(c, "Duplicate property name %s in %s",
+				FAIL(c, dti, "Duplicate property name %s in %s",
 				     prop->name, node->fullpath);
 		}
 	}
@@ -247,7 +250,7 @@ static void check_node_name_chars(struct check *c, struct dt_info *dti,
 	int n = strspn(node->name, c->data);
 
 	if (n < strlen(node->name))
-		FAIL(c, "Bad character '%c' in node %s",
+		FAIL(c, dti, "Bad character '%c' in node %s",
 		     node->name[n], node->fullpath);
 }
 ERROR(node_name_chars, check_node_name_chars, PROPNODECHARS "@");
@@ -258,7 +261,7 @@ static void check_node_name_chars_strict(struct check *c, struct dt_info *dti,
 	int n = strspn(node->name, c->data);
 
 	if (n < node->basenamelen)
-		FAIL(c, "Character '%c' not recommended in node %s",
+		FAIL(c, dti, "Character '%c' not recommended in node %s",
 		     node->name[n], node->fullpath);
 }
 CHECK(node_name_chars_strict, check_node_name_chars_strict, PROPNODECHARSSTRICT);
@@ -267,7 +270,7 @@ static void check_node_name_format(struct check *c, struct dt_info *dti,
 				   struct node *node)
 {
 	if (strchr(get_unitname(node), '@'))
-		FAIL(c, "Node %s has multiple '@' characters in name",
+		FAIL(c, dti, "Node %s has multiple '@' characters in name",
 		     node->fullpath);
 }
 ERROR(node_name_format, check_node_name_format, NULL, &node_name_chars);
@@ -286,11 +289,11 @@ static void check_unit_address_vs_reg(struct check *c, struct dt_info *dti,
 
 	if (prop) {
 		if (!unitname[0])
-			FAIL(c, "Node %s has a reg or ranges property, but no unit name",
+			FAIL(c, dti, "Node %s has a reg or ranges property, but no unit name",
 			    node->fullpath);
 	} else {
 		if (unitname[0])
-			FAIL(c, "Node %s has a unit name, but no reg property",
+			FAIL(c, dti, "Node %s has a unit name, but no reg property",
 			    node->fullpath);
 	}
 }
@@ -305,7 +308,7 @@ static void check_property_name_chars(struct check *c, struct dt_info *dti,
 		int n = strspn(prop->name, c->data);
 
 		if (n < strlen(prop->name))
-			FAIL(c, "Bad character '%c' in property name \"%s\", node %s",
+			FAIL(c, dti, "Bad character '%c' in property name \"%s\", node %s",
 			     prop->name[n], prop->name, node->fullpath);
 	}
 }
@@ -337,7 +340,7 @@ static void check_property_name_chars_strict(struct check *c,
 			n = strspn(name, c->data);
 		}
 		if (n < strlen(name))
-			FAIL(c, "Character '%c' not recommended in property name \"%s\", node %s",
+			FAIL(c, dti, "Character '%c' not recommended in property name \"%s\", node %s",
 			     name[n], prop->name, node->fullpath);
 	}
 }
@@ -371,7 +374,7 @@ static void check_duplicate_label(struct check *c, struct dt_info *dti,
 		return;
 
 	if ((othernode != node) || (otherprop != prop) || (othermark != mark))
-		FAIL(c, "Duplicate label '%s' on " DESCLABEL_FMT
+		FAIL(c, dti, "Duplicate label '%s' on " DESCLABEL_FMT
 		     " and " DESCLABEL_FMT,
 		     label, DESCLABEL_ARGS(node, prop, mark),
 		     DESCLABEL_ARGS(othernode, otherprop, othermark));
@@ -411,7 +414,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
 		return 0;
 
 	if (prop->val.len != sizeof(cell_t)) {
-		FAIL(c, "%s has bad length (%d) %s property",
+		FAIL(c, dti, "%s has bad length (%d) %s property",
 		     node->fullpath, prop->val.len, prop->name);
 		return 0;
 	}
@@ -423,7 +426,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
 			/* "Set this node's phandle equal to some
 			 * other node's phandle".  That's nonsensical
 			 * by construction. */ {
-			FAIL(c, "%s in %s is a reference to another node",
+			FAIL(c, dti, "%s in %s is a reference to another node",
 			     prop->name, node->fullpath);
 		}
 		/* But setting this node's phandle equal to its own
@@ -437,7 +440,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
 	phandle = propval_cell(prop);
 
 	if ((phandle == 0) || (phandle == -1)) {
-		FAIL(c, "%s has bad value (0x%x) in %s property",
+		FAIL(c, dti, "%s has bad value (0x%x) in %s property",
 		     node->fullpath, phandle, prop->name);
 		return 0;
 	}
@@ -464,7 +467,7 @@ static void check_explicit_phandles(struct check *c, struct dt_info *dti,
 		return;
 
 	if (linux_phandle && phandle && (phandle != linux_phandle))
-		FAIL(c, "%s has mismatching 'phandle' and 'linux,phandle'"
+		FAIL(c, dti, "%s has mismatching 'phandle' and 'linux,phandle'"
 		     " properties", node->fullpath);
 
 	if (linux_phandle && !phandle)
@@ -472,7 +475,7 @@ static void check_explicit_phandles(struct check *c, struct dt_info *dti,
 
 	other = get_node_by_phandle(root, phandle);
 	if (other && (other != node)) {
-		FAIL(c, "%s has duplicated phandle 0x%x (seen before at %s)",
+		FAIL(c, dti, "%s has duplicated phandle 0x%x (seen before at %s)",
 		     node->fullpath, phandle, other->fullpath);
 		return;
 	}
@@ -497,7 +500,7 @@ static void check_name_properties(struct check *c, struct dt_info *dti,
 
 	if ((prop->val.len != node->basenamelen+1)
 	    || (memcmp(prop->val.val, node->name, node->basenamelen) != 0)) {
-		FAIL(c, "\"name\" property in %s is incorrect (\"%s\" instead"
+		FAIL(c, dti, "\"name\" property in %s is incorrect (\"%s\" instead"
 		     " of base node name)", node->fullpath, prop->val.val);
 	} else {
 		/* The name property is correct, and therefore redundant.
@@ -532,7 +535,7 @@ static void fixup_phandle_references(struct check *c, struct dt_info *dti,
 			refnode = get_node_by_ref(dt, m->ref);
 			if (! refnode) {
 				if (!(dti->dtsflags & DTSF_PLUGIN))
-					FAIL(c, "Reference to non-existent node or "
+					FAIL(c, dti, "Reference to non-existent node or "
 							"label \"%s\"\n", m->ref);
 				else /* mark the entry as unresolved */
 					*((cell_t *)(prop->val.val + m->offset)) =
@@ -564,7 +567,7 @@ static void fixup_path_references(struct check *c, struct dt_info *dti,
 
 			refnode = get_node_by_ref(dt, m->ref);
 			if (!refnode) {
-				FAIL(c, "Reference to non-existent node or label \"%s\"\n",
+				FAIL(c, dti, "Reference to non-existent node or label \"%s\"\n",
 				     m->ref);
 				continue;
 			}
@@ -623,19 +626,19 @@ static void check_reg_format(struct check *c, struct dt_info *dti,
 		return; /* No "reg", that's fine */
 
 	if (!node->parent) {
-		FAIL(c, "Root node has a \"reg\" property");
+		FAIL(c, dti, "Root node has a \"reg\" property");
 		return;
 	}
 
 	if (prop->val.len == 0)
-		FAIL(c, "\"reg\" property in %s is empty", node->fullpath);
+		FAIL(c, dti, "\"reg\" property in %s is empty", node->fullpath);
 
 	addr_cells = node_addr_cells(node->parent);
 	size_cells = node_size_cells(node->parent);
 	entrylen = (addr_cells + size_cells) * sizeof(cell_t);
 
 	if (!entrylen || (prop->val.len % entrylen) != 0)
-		FAIL(c, "\"reg\" property in %s has invalid length (%d bytes) "
+		FAIL(c, dti, "\"reg\" property in %s has invalid length (%d bytes) "
 		     "(#address-cells == %d, #size-cells == %d)",
 		     node->fullpath, prop->val.len, addr_cells, size_cells);
 }
@@ -652,7 +655,7 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
 		return;
 
 	if (!node->parent) {
-		FAIL(c, "Root node has a \"ranges\" property");
+		FAIL(c, dti, "Root node has a \"ranges\" property");
 		return;
 	}
 
@@ -664,17 +667,17 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
 
 	if (prop->val.len == 0) {
 		if (p_addr_cells != c_addr_cells)
-			FAIL(c, "%s has empty \"ranges\" property but its "
+			FAIL(c, dti, "%s has empty \"ranges\" property but its "
 			     "#address-cells (%d) differs from %s (%d)",
 			     node->fullpath, c_addr_cells, node->parent->fullpath,
 			     p_addr_cells);
 		if (p_size_cells != c_size_cells)
-			FAIL(c, "%s has empty \"ranges\" property but its "
+			FAIL(c, dti, "%s has empty \"ranges\" property but its "
 			     "#size-cells (%d) differs from %s (%d)",
 			     node->fullpath, c_size_cells, node->parent->fullpath,
 			     p_size_cells);
 	} else if ((prop->val.len % entrylen) != 0) {
-		FAIL(c, "\"ranges\" property in %s has invalid length (%d bytes) "
+		FAIL(c, dti, "\"ranges\" property in %s has invalid length (%d bytes) "
 		     "(parent #address-cells == %d, child #address-cells == %d, "
 		     "#size-cells == %d)", node->fullpath, prop->val.len,
 		     p_addr_cells, c_addr_cells, c_size_cells);
@@ -700,11 +703,11 @@ static void check_avoid_default_addr_size(struct check *c, struct dt_info *dti,
 		return;
 
 	if (node->parent->addr_cells == -1)
-		FAIL(c, "Relying on default #address-cells value for %s",
+		FAIL(c, dti, "Relying on default #address-cells value for %s",
 		     node->fullpath);
 
 	if (node->parent->size_cells == -1)
-		FAIL(c, "Relying on default #size-cells value for %s",
+		FAIL(c, dti, "Relying on default #size-cells value for %s",
 		     node->fullpath);
 }
 WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
@@ -728,7 +731,7 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
 
 	prop = get_property(chosen, "interrupt-controller");
 	if (prop)
-		FAIL(c, "/chosen has obsolete \"interrupt-controller\" "
+		FAIL(c, dti, "/chosen has obsolete \"interrupt-controller\" "
 		     "property");
 }
 WARNING(obsolete_chosen_interrupt_controller,
diff --git a/dtc.c b/dtc.c
index a4edf4c..9c30c33 100644
--- a/dtc.c
+++ b/dtc.c
@@ -309,6 +309,8 @@ int main(int argc, char *argv[])
 	else
 		die("Unknown input format \"%s\"\n", inform);
 
+	dti->outname = outname;
+
 	if (depfile) {
 		fputc('\n', depfile);
 		fclose(depfile);
diff --git a/dtc.h b/dtc.h
index c6f125c..1ac2a1e 100644
--- a/dtc.h
+++ b/dtc.h
@@ -246,6 +246,7 @@ struct dt_info {
 	struct reserve_info *reservelist;
 	uint32_t boot_cpuid_phys;
 	struct node *dt;		/* the device tree */
+	const char *outname;		/* filename being written to, "-" for stdout */
 };
 
 /* DTS version flags definitions */
-- 
2.11.0

Re: Warnings do include offending filename

[David Gibson]

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

On Thu, Feb 23, 2017 at 08:44:00AM +0000, Ian Campbell wrote:
> On Thu, 2017-02-23 at 14:42 +1100, David Gibson wrote:
> > Ok looks good.  Except that since you wrote it I've merged some other
> > patches adding more FAIL() calls, so this now causes compile errors.
> > 
> > Can you please rebase onto current master, fix up for the new calls
> > and resend.
> 
> Done.

Thanks, applied.

> 
> 8----------
> 
> >From 011d25c56ba77f1050c4c9d1f15b531ce0a4ab9d Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ijc-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
> Date: Fri, 3 Feb 2017 08:29:39 +0000
> Subject: [PATCH] Print output filename as part of warning messages
> 
> For example:
> src/arm/at91-ariag25.dtb: Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
> 
> If output is to stdout then the prefix is "<stdout>: ".
> 
> This helps to direct the developer to where to look when multiple files are
> being compiled in parallel.
> 
> Signed-off-by: Ian Campbell <ijc-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
> ---
>  checks.c | 83 +++++++++++++++++++++++++++++++++-------------------------------
>  dtc.c    |  2 ++
>  dtc.h    |  1 +
>  3 files changed, 46 insertions(+), 40 deletions(-)
> 
> diff --git a/checks.c b/checks.c
> index cce0cca..0e8b978 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -73,16 +73,19 @@ struct check {
>  	CHECK_ENTRY(_nm, _fn, _d, false, false, __VA_ARGS__)
>  
>  #ifdef __GNUC__
> -static inline void check_msg(struct check *c, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
> +static inline void check_msg(struct check *c, struct dt_info *dti,
> +			     const char *fmt, ...) __attribute__((format (printf, 3, 4)));
>  #endif
> -static inline void check_msg(struct check *c, const char *fmt, ...)
> +static inline void check_msg(struct check *c, struct dt_info *dti,
> +			     const char *fmt, ...)
>  {
>  	va_list ap;
>  	va_start(ap, fmt);
>  
>  	if ((c->warn && (quiet < 1))
>  	    || (c->error && (quiet < 2))) {
> -		fprintf(stderr, "%s (%s): ",
> +		fprintf(stderr, "%s: %s (%s): ",
> +			strcmp(dti->outname, "-") ? dti->outname : "<stdout>",
>  			(c->error) ? "ERROR" : "Warning", c->name);
>  		vfprintf(stderr, fmt, ap);
>  		fprintf(stderr, "\n");
> @@ -90,11 +93,11 @@ static inline void check_msg(struct check *c, const char *fmt, ...)
>  	va_end(ap);
>  }
>  
> -#define FAIL(c, ...) \
> -	do { \
> -		TRACE((c), "\t\tFAILED at %s:%d", __FILE__, __LINE__); \
> -		(c)->status = FAILED; \
> -		check_msg((c), __VA_ARGS__); \
> +#define FAIL(c, dti, ...)						\
> +	do {								\
> +		TRACE((c), "\t\tFAILED at %s:%d", __FILE__, __LINE__);	\
> +		(c)->status = FAILED;					\
> +		check_msg((c), dti, __VA_ARGS__);			\
>  	} while (0)
>  
>  static void check_nodes_props(struct check *c, struct dt_info *dti, struct node *node)
> @@ -127,7 +130,7 @@ static bool run_check(struct check *c, struct dt_info *dti)
>  		error = error || run_check(prq, dti);
>  		if (prq->status != PASSED) {
>  			c->status = PREREQ;
> -			check_msg(c, "Failed prerequisite '%s'",
> +			check_msg(c, dti, "Failed prerequisite '%s'",
>  				  c->prereq[i]->name);
>  		}
>  	}
> @@ -157,7 +160,7 @@ out:
>  static inline void check_always_fail(struct check *c, struct dt_info *dti,
>  				     struct node *node)
>  {
> -	FAIL(c, "always_fail check");
> +	FAIL(c, dti, "always_fail check");
>  }
>  CHECK(always_fail, check_always_fail, NULL);
>  
> @@ -172,7 +175,7 @@ static void check_is_string(struct check *c, struct dt_info *dti,
>  		return; /* Not present, assumed ok */
>  
>  	if (!data_is_one_string(prop->val))
> -		FAIL(c, "\"%s\" property in %s is not a string",
> +		FAIL(c, dti, "\"%s\" property in %s is not a string",
>  		     propname, node->fullpath);
>  }
>  #define WARNING_IF_NOT_STRING(nm, propname) \
> @@ -191,7 +194,7 @@ static void check_is_cell(struct check *c, struct dt_info *dti,
>  		return; /* Not present, assumed ok */
>  
>  	if (prop->val.len != sizeof(cell_t))
> -		FAIL(c, "\"%s\" property in %s is not a single cell",
> +		FAIL(c, dti, "\"%s\" property in %s is not a single cell",
>  		     propname, node->fullpath);
>  }
>  #define WARNING_IF_NOT_CELL(nm, propname) \
> @@ -213,7 +216,7 @@ static void check_duplicate_node_names(struct check *c, struct dt_info *dti,
>  		     child2;
>  		     child2 = child2->next_sibling)
>  			if (streq(child->name, child2->name))
> -				FAIL(c, "Duplicate node name %s",
> +				FAIL(c, dti, "Duplicate node name %s",
>  				     child->fullpath);
>  }
>  ERROR(duplicate_node_names, check_duplicate_node_names, NULL);
> @@ -228,7 +231,7 @@ static void check_duplicate_property_names(struct check *c, struct dt_info *dti,
>  			if (prop2->deleted)
>  				continue;
>  			if (streq(prop->name, prop2->name))
> -				FAIL(c, "Duplicate property name %s in %s",
> +				FAIL(c, dti, "Duplicate property name %s in %s",
>  				     prop->name, node->fullpath);
>  		}
>  	}
> @@ -247,7 +250,7 @@ static void check_node_name_chars(struct check *c, struct dt_info *dti,
>  	int n = strspn(node->name, c->data);
>  
>  	if (n < strlen(node->name))
> -		FAIL(c, "Bad character '%c' in node %s",
> +		FAIL(c, dti, "Bad character '%c' in node %s",
>  		     node->name[n], node->fullpath);
>  }
>  ERROR(node_name_chars, check_node_name_chars, PROPNODECHARS "@");
> @@ -258,7 +261,7 @@ static void check_node_name_chars_strict(struct check *c, struct dt_info *dti,
>  	int n = strspn(node->name, c->data);
>  
>  	if (n < node->basenamelen)
> -		FAIL(c, "Character '%c' not recommended in node %s",
> +		FAIL(c, dti, "Character '%c' not recommended in node %s",
>  		     node->name[n], node->fullpath);
>  }
>  CHECK(node_name_chars_strict, check_node_name_chars_strict, PROPNODECHARSSTRICT);
> @@ -267,7 +270,7 @@ static void check_node_name_format(struct check *c, struct dt_info *dti,
>  				   struct node *node)
>  {
>  	if (strchr(get_unitname(node), '@'))
> -		FAIL(c, "Node %s has multiple '@' characters in name",
> +		FAIL(c, dti, "Node %s has multiple '@' characters in name",
>  		     node->fullpath);
>  }
>  ERROR(node_name_format, check_node_name_format, NULL, &node_name_chars);
> @@ -286,11 +289,11 @@ static void check_unit_address_vs_reg(struct check *c, struct dt_info *dti,
>  
>  	if (prop) {
>  		if (!unitname[0])
> -			FAIL(c, "Node %s has a reg or ranges property, but no unit name",
> +			FAIL(c, dti, "Node %s has a reg or ranges property, but no unit name",
>  			    node->fullpath);
>  	} else {
>  		if (unitname[0])
> -			FAIL(c, "Node %s has a unit name, but no reg property",
> +			FAIL(c, dti, "Node %s has a unit name, but no reg property",
>  			    node->fullpath);
>  	}
>  }
> @@ -305,7 +308,7 @@ static void check_property_name_chars(struct check *c, struct dt_info *dti,
>  		int n = strspn(prop->name, c->data);
>  
>  		if (n < strlen(prop->name))
> -			FAIL(c, "Bad character '%c' in property name \"%s\", node %s",
> +			FAIL(c, dti, "Bad character '%c' in property name \"%s\", node %s",
>  			     prop->name[n], prop->name, node->fullpath);
>  	}
>  }
> @@ -337,7 +340,7 @@ static void check_property_name_chars_strict(struct check *c,
>  			n = strspn(name, c->data);
>  		}
>  		if (n < strlen(name))
> -			FAIL(c, "Character '%c' not recommended in property name \"%s\", node %s",
> +			FAIL(c, dti, "Character '%c' not recommended in property name \"%s\", node %s",
>  			     name[n], prop->name, node->fullpath);
>  	}
>  }
> @@ -371,7 +374,7 @@ static void check_duplicate_label(struct check *c, struct dt_info *dti,
>  		return;
>  
>  	if ((othernode != node) || (otherprop != prop) || (othermark != mark))
> -		FAIL(c, "Duplicate label '%s' on " DESCLABEL_FMT
> +		FAIL(c, dti, "Duplicate label '%s' on " DESCLABEL_FMT
>  		     " and " DESCLABEL_FMT,
>  		     label, DESCLABEL_ARGS(node, prop, mark),
>  		     DESCLABEL_ARGS(othernode, otherprop, othermark));
> @@ -411,7 +414,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
>  		return 0;
>  
>  	if (prop->val.len != sizeof(cell_t)) {
> -		FAIL(c, "%s has bad length (%d) %s property",
> +		FAIL(c, dti, "%s has bad length (%d) %s property",
>  		     node->fullpath, prop->val.len, prop->name);
>  		return 0;
>  	}
> @@ -423,7 +426,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
>  			/* "Set this node's phandle equal to some
>  			 * other node's phandle".  That's nonsensical
>  			 * by construction. */ {
> -			FAIL(c, "%s in %s is a reference to another node",
> +			FAIL(c, dti, "%s in %s is a reference to another node",
>  			     prop->name, node->fullpath);
>  		}
>  		/* But setting this node's phandle equal to its own
> @@ -437,7 +440,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti,
>  	phandle = propval_cell(prop);
>  
>  	if ((phandle == 0) || (phandle == -1)) {
> -		FAIL(c, "%s has bad value (0x%x) in %s property",
> +		FAIL(c, dti, "%s has bad value (0x%x) in %s property",
>  		     node->fullpath, phandle, prop->name);
>  		return 0;
>  	}
> @@ -464,7 +467,7 @@ static void check_explicit_phandles(struct check *c, struct dt_info *dti,
>  		return;
>  
>  	if (linux_phandle && phandle && (phandle != linux_phandle))
> -		FAIL(c, "%s has mismatching 'phandle' and 'linux,phandle'"
> +		FAIL(c, dti, "%s has mismatching 'phandle' and 'linux,phandle'"
>  		     " properties", node->fullpath);
>  
>  	if (linux_phandle && !phandle)
> @@ -472,7 +475,7 @@ static void check_explicit_phandles(struct check *c, struct dt_info *dti,
>  
>  	other = get_node_by_phandle(root, phandle);
>  	if (other && (other != node)) {
> -		FAIL(c, "%s has duplicated phandle 0x%x (seen before at %s)",
> +		FAIL(c, dti, "%s has duplicated phandle 0x%x (seen before at %s)",
>  		     node->fullpath, phandle, other->fullpath);
>  		return;
>  	}
> @@ -497,7 +500,7 @@ static void check_name_properties(struct check *c, struct dt_info *dti,
>  
>  	if ((prop->val.len != node->basenamelen+1)
>  	    || (memcmp(prop->val.val, node->name, node->basenamelen) != 0)) {
> -		FAIL(c, "\"name\" property in %s is incorrect (\"%s\" instead"
> +		FAIL(c, dti, "\"name\" property in %s is incorrect (\"%s\" instead"
>  		     " of base node name)", node->fullpath, prop->val.val);
>  	} else {
>  		/* The name property is correct, and therefore redundant.
> @@ -532,7 +535,7 @@ static void fixup_phandle_references(struct check *c, struct dt_info *dti,
>  			refnode = get_node_by_ref(dt, m->ref);
>  			if (! refnode) {
>  				if (!(dti->dtsflags & DTSF_PLUGIN))
> -					FAIL(c, "Reference to non-existent node or "
> +					FAIL(c, dti, "Reference to non-existent node or "
>  							"label \"%s\"\n", m->ref);
>  				else /* mark the entry as unresolved */
>  					*((cell_t *)(prop->val.val + m->offset)) =
> @@ -564,7 +567,7 @@ static void fixup_path_references(struct check *c, struct dt_info *dti,
>  
>  			refnode = get_node_by_ref(dt, m->ref);
>  			if (!refnode) {
> -				FAIL(c, "Reference to non-existent node or label \"%s\"\n",
> +				FAIL(c, dti, "Reference to non-existent node or label \"%s\"\n",
>  				     m->ref);
>  				continue;
>  			}
> @@ -623,19 +626,19 @@ static void check_reg_format(struct check *c, struct dt_info *dti,
>  		return; /* No "reg", that's fine */
>  
>  	if (!node->parent) {
> -		FAIL(c, "Root node has a \"reg\" property");
> +		FAIL(c, dti, "Root node has a \"reg\" property");
>  		return;
>  	}
>  
>  	if (prop->val.len == 0)
> -		FAIL(c, "\"reg\" property in %s is empty", node->fullpath);
> +		FAIL(c, dti, "\"reg\" property in %s is empty", node->fullpath);
>  
>  	addr_cells = node_addr_cells(node->parent);
>  	size_cells = node_size_cells(node->parent);
>  	entrylen = (addr_cells + size_cells) * sizeof(cell_t);
>  
>  	if (!entrylen || (prop->val.len % entrylen) != 0)
> -		FAIL(c, "\"reg\" property in %s has invalid length (%d bytes) "
> +		FAIL(c, dti, "\"reg\" property in %s has invalid length (%d bytes) "
>  		     "(#address-cells == %d, #size-cells == %d)",
>  		     node->fullpath, prop->val.len, addr_cells, size_cells);
>  }
> @@ -652,7 +655,7 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
>  		return;
>  
>  	if (!node->parent) {
> -		FAIL(c, "Root node has a \"ranges\" property");
> +		FAIL(c, dti, "Root node has a \"ranges\" property");
>  		return;
>  	}
>  
> @@ -664,17 +667,17 @@ static void check_ranges_format(struct check *c, struct dt_info *dti,
>  
>  	if (prop->val.len == 0) {
>  		if (p_addr_cells != c_addr_cells)
> -			FAIL(c, "%s has empty \"ranges\" property but its "
> +			FAIL(c, dti, "%s has empty \"ranges\" property but its "
>  			     "#address-cells (%d) differs from %s (%d)",
>  			     node->fullpath, c_addr_cells, node->parent->fullpath,
>  			     p_addr_cells);
>  		if (p_size_cells != c_size_cells)
> -			FAIL(c, "%s has empty \"ranges\" property but its "
> +			FAIL(c, dti, "%s has empty \"ranges\" property but its "
>  			     "#size-cells (%d) differs from %s (%d)",
>  			     node->fullpath, c_size_cells, node->parent->fullpath,
>  			     p_size_cells);
>  	} else if ((prop->val.len % entrylen) != 0) {
> -		FAIL(c, "\"ranges\" property in %s has invalid length (%d bytes) "
> +		FAIL(c, dti, "\"ranges\" property in %s has invalid length (%d bytes) "
>  		     "(parent #address-cells == %d, child #address-cells == %d, "
>  		     "#size-cells == %d)", node->fullpath, prop->val.len,
>  		     p_addr_cells, c_addr_cells, c_size_cells);
> @@ -700,11 +703,11 @@ static void check_avoid_default_addr_size(struct check *c, struct dt_info *dti,
>  		return;
>  
>  	if (node->parent->addr_cells == -1)
> -		FAIL(c, "Relying on default #address-cells value for %s",
> +		FAIL(c, dti, "Relying on default #address-cells value for %s",
>  		     node->fullpath);
>  
>  	if (node->parent->size_cells == -1)
> -		FAIL(c, "Relying on default #size-cells value for %s",
> +		FAIL(c, dti, "Relying on default #size-cells value for %s",
>  		     node->fullpath);
>  }
>  WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
> @@ -728,7 +731,7 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
>  
>  	prop = get_property(chosen, "interrupt-controller");
>  	if (prop)
> -		FAIL(c, "/chosen has obsolete \"interrupt-controller\" "
> +		FAIL(c, dti, "/chosen has obsolete \"interrupt-controller\" "
>  		     "property");
>  }
>  WARNING(obsolete_chosen_interrupt_controller,
> diff --git a/dtc.c b/dtc.c
> index a4edf4c..9c30c33 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -309,6 +309,8 @@ int main(int argc, char *argv[])
>  	else
>  		die("Unknown input format \"%s\"\n", inform);
>  
> +	dti->outname = outname;
> +
>  	if (depfile) {
>  		fputc('\n', depfile);
>  		fclose(depfile);
> diff --git a/dtc.h b/dtc.h
> index c6f125c..1ac2a1e 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -246,6 +246,7 @@ struct dt_info {
>  	struct reserve_info *reservelist;
>  	uint32_t boot_cpuid_phys;
>  	struct node *dt;		/* the device tree */
> +	const char *outname;		/* filename being written to, "-" for stdout */
>  };
>  
>  /* DTS version flags definitions */

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]