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 <[email protected]>
> ---
>  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 <[email protected]>

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(-)
_______________________________________________
xorg-driver-ati mailing list
[email protected]
http://lists.x.org/mailman/listinfo/xorg-driver-ati

Reply via email to