[PATCH radeontool 1/5] Clearer error message on fatal errors

Jonathan Nieder jrnieder at gmail.com
Fri Dec 2 14:34:43 PST 2011


Hi Tormod,

Tormod Volden wrote:

> Let the user know that we actually hit an error.
> This fixes gcc format-security warnings as well.
>
> Also add a missing newline in one error message.
>
> Signed-off-by: Tormod Volden <debian.tormod at gmail.com>
> ---
>  avivotool.c  |    2 +-
>  radeonreg.c  |    2 +-
>  radeontool.c |    4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)

I like all three effects.

Prefixing messages with "Error:" makes it clearer why exactly
radeontool died.  A tiny nitpick is that it would be nice if "Error:"
and "usage:" had the same capitalization.  It also might make sense to
provide a fatalf() variadic function, to allow messages like

	write died: Resource temporarily unavailable
	Error: writing to stdout
	$

to take up only one line.

gcc doesn't seem to realize that all current callers to fatal()
provide constants.  Passing "%s" to avoid tripping -Wformat-security
seems like an unambiguously good thing. :)

Missing newlines in error messages would be easier to avoid if fatal()
took care of the newline.

So maybe something like the following would make sense.  With or
without these tweaks,

Reviewed-by: Jonathan Nieder <jrnieder at gmail.com>

Jonathan Nieder (2):
  avoid -Wformat-security warnings
  teach fatal() to write newline

Tormod Volden (2):
  radeontool: add missing newline to error message
  prefix fatal error messages with "fatal error:"

 avivotool.c  |   28 ++++++++++++++--------------
 radeonreg.c  |   14 +++++++-------
 radeontool.c |   20 ++++++++++----------
 3 files changed, 31 insertions(+), 31 deletions(-)


More information about the xorg-driver-ati mailing list