[Intel-gfx] [PATCH i-g-t] intel_l3_parity: More helpful output in case of errors

[Daniel Vetter]

On Wed, Sep 06, 2017 at 12:31:05PM +0300, Petri Latvala wrote:
> 
> 
> On 09/05/2017 07:16 PM, Daniel Vetter wrote:
> > On Tue, Sep 05, 2017 at 03:39:49PM +0300, Petri Latvala wrote:
> > > When no action is specified on the command line, print the usage help
> > > text and exit with failure instead of SIGABRT. Fix some typos on the
> > > usage text.
> > > 
> > > Keep the abort() call in places where they can only be reached by
> > > expanding the tool and forgetting to handle new parameters, with an
> > > error message printed.
> > > 
> > > CC: Ben Widawsky <benjamin.widawsky at intel.com>
> > > Signed-off-by: Petri Latvala <petri.latvala at intel.com>
> > > ---
> > >   tools/intel_l3_parity.c | 12 +++++++++---
> > >   1 file changed, 9 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
> > > index eb00c50..1a4fae5 100644
> > > --- a/tools/intel_l3_parity.c
> > > +++ b/tools/intel_l3_parity.c
> > > @@ -172,9 +172,9 @@ static void usage(const char *name)
> > >   		"  -l, --list				List the current L3 logs\n"
> > >   		"  -a, --clear-all			Clear all disabled rows\n"
> > >   		"  -e, --enable				Enable row, bank, subbank (undo -d)\n"
> > > -		"  -d, --disable=<row,bank,subbank>	Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead\n"
> > > -		"  -i, --inject				[HSW only] Cause hardware to inject a row errors\n"
> > > -		"  -u, --uninject			[HSW only] Turn off hardware error injectection (undo -i)\n"
> > > +		"  -d, --disable=<row,bank,subbank>	Disable row, bank, subbank (inline arguments are deprecated. Please use -r, -b, -s instead)\n"
> > > +		"  -i, --inject				[HSW only] Cause hardware to inject a row error\n"
> > > +		"  -u, --uninject			[HSW only] Turn off hardware error injection (undo -i)\n"
> > >   		"  -L, --listen				Listen for uevent errors\n",
> > >   		name);
> > >   }
> > > @@ -301,6 +301,7 @@ int main(int argc, char *argv[])
> > >   				action = c;
> > >   				break;
> > >   			default:
> > > +				fprintf(stderr, "Internal error: Unhandled flag %c\n", c);
> > >   				abort();
> > >   		}
> > >   	}
> > > @@ -374,7 +375,12 @@ int main(int argc, char *argv[])
> > >   				break;
> > >   			case 'L':
> > >   				break;
> > > +			case '0':
> > > +				/* No action given */
> > > +				usage(argv[0]);
> > > +				exit(EXIT_FAILURE);
> > Won't this print usage once per slice? Or am I misreading how the patch
> > applies ...
> 
> 
> It prints the usage and calls exit(), what is the control flow that leads to
> printing it multiple times?

Ah, that's indeed a bit confusing control flow that usage() exists. Would
be cleaner if we don't hide the call somewhere in a loop, but bail out
more top-level.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch