[flashrom] [PATCH] Add logfile support to flashrom

[Uwe Hermann]

On Thu, Jun 09, 2011 at 10:32:30PM +0200, Carl-Daniel Hailfinger wrote:
> Add log file support to flashrom.
> 
> The log file will always contain all verbose messages even if the user
> doesn't specify -V. If the user specifies -VV, SPEW messages will be
> logged as well.
> 
> Proof of concept, comments welcome.
> 
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
> 
> Index: flashrom-logfile/flash.h
> ===================================================================
> --- flashrom-logfile/flash.h	(Revision 1326)
> +++ flashrom-logfile/flash.h	(Arbeitskopie)
> @@ -229,6 +229,8 @@
>  #define ERROR_NONFATAL 0x100
>  
>  /* cli_output.c */
> +int open_logfile(char *filename);

Can be "const char" too, I guess.


> +int msg_log(const char *fmt, ...);
>  /* Let gcc and clang check for correct printf-style format strings. */
>  int print(int type, const char *fmt, ...) __attribute__((format(printf, 2, 3)));
>  #define MSG_ERROR	0
> Index: flashrom-logfile/cli_output.c
> ===================================================================
> --- flashrom-logfile/cli_output.c	(Revision 1326)
> +++ flashrom-logfile/cli_output.c	(Arbeitskopie)
> @@ -2,6 +2,7 @@
>   * This file is part of the flashrom project.
>   *
>   * Copyright (C) 2009 Sean Nelson <audiohacked at gmail.com>
> + * Copyright (C) 2011 Carl-Daniel Hailfinger
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -22,30 +23,70 @@
>  #include <stdarg.h>
>  #include "flash.h"
>  
> -int print(int type, const char *fmt, ...)
> +static FILE *logfile = NULL;
> +
> +int open_logfile(char *filename)
>  {
> +	if (!filename) {
> +		msg_gerr("No filename specified.\n");
> +		return 1;
> +	}
> +	if ((logfile = fopen(filename, "w")) == NULL) {

Use "wb" please, the "b" is required to avoid issues on Windows. There's
another occurence in flashrom of fopen() withouth "b", that should be
fixed too.


> +int print(int type, const char *fmt, ...)
> +{
> +	va_list ap;

> +	int ret = 0;
> +	int want_screen = 1;
> +	int want_file = 1;

Can be merged in one line for more compact code.


There's a small issue with the patch, it double-prints some lines on stdout
now (in the logfile they appear only once if -o is used):

  $ ./flashrom 
  flashrom v0.9.3-r1331 on Linux 2.6.38-2-amd64 (x86_64), built with libpci 3.1.7, GCC 4.5.2, little endian
  flashrom is free software, get the source code at http://www.flashrom.org
  
  flashrom v0.9.3-r1331 on Linux 2.6.38-2-amd64 (x86_64), built with libpci 3.1.7, GCC 4.5.2, little endian
  Calibrating delay loop... OK.
  ERROR: Could not get I/O privileges (Operation not permitted).
  You need to be root.

Also, not all whitespace seems to be the same as in the logfile, e.g. for

  $ ./flashrom -L -o foo


Uwe.
-- 
http://hermann-uwe.de     | http://sigrok.org
http://randomprojects.org | http://unmaintained-free-software.org