---
pyanaconda/isys/log.c | 55 +++++++++++++++++++++++++-----------------------
1 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/pyanaconda/isys/log.c b/pyanaconda/isys/log.c
index 96da57e..0a45bf3 100644
--- a/pyanaconda/isys/log.c
+++ b/pyanaconda/isys/log.c
@@ -27,6 +27,7 @@
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
+#include <string.h>
#include <time.h>
#include <unistd.h>
#include <sys/time.h>
@@ -62,40 +63,42 @@ static int mapLogLevel(int level)
}
}

+static void log_level_to_str(const int level, char* outbuf)
+{
+ switch (level) {
+ case DEBUGLVL:
+ strcpy(outbuf, "DEBUG");
+ break;
+ case INFO:
+ strcpy(outbuf, "INFO");
+ break;
+ case WARNING:
+ strcpy(outbuf, "WARN");
+ break;
+ case ERROR:
+ strcpy(outbuf, "ERR");
+ break;
+ case CRITICAL:
+ strcpy(outbuf, "CRIT");
+ break;
+ default:
+ strcpy(outbuf, "(UNKNWN)");
+ break;
+ }
+}
+
static void printLogHeader(int level, const char *tag, FILE *outfile) {
struct timeval current_time;
struct tm *t;
int msecs;
+ char level_name[10];

gettimeofday(&current_time, NULL);
t = gmtime(&current_time.tv_sec);
msecs = current_time.tv_usec / 1000;
- switch (level) {
- case DEBUGLVL:
- fprintf (outfile, "%02d:%02d:%02d,%03d DEBUG %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case INFO:
- fprintf (outfile, "%02d:%02d:%02d,%03d INFO %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case WARNING:
- fprintf (outfile, "%02d:%02d:%02d,%03d WARN %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case ERROR:
- fprintf (outfile, "%02d:%02d:%02d,%03d ERR %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case CRITICAL:
- fprintf (outfile, "%02d:%02d:%02d,%03d CRIT %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
- }
+ log_level_to_str(level, level_name);
+ fprintf(outfile, "%02d:%02d:%02d,%03d %s %s: ", t->tm_hour,
+ t->tm_min, t->tm_sec, msecs, level_name, tag);
}

static void printLogMessage(int level, const char *tag, FILE *outfile, const char *s, va_list ap)
--
1.7.1.1

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

const char log_level_to_str = { ..., ... }
and keep it in sync with constant definitions of DEBUGLEVEL et al.?

On 08/13/2010 03:30 PM, Ales Kozumplik wrote:
> ---
> pyanaconda/isys/log.c | 55 +++++++++++++++++++++++++-----------------------
> 1 files changed, 29 insertions(+), 26 deletions(-)
>
> diff --git a/pyanaconda/isys/log.c b/pyanaconda/isys/log.c
> index 96da57e..0a45bf3 100644
> --- a/pyanaconda/isys/log.c
> +++ b/pyanaconda/isys/log.c
> @@ -27,6 +27,7 @@
> #include <stdarg.h>
> #include <stdio.h>
> #include <stdlib.h>
> +#include <string.h>
> #include <time.h>
> #include <unistd.h>
> #include <sys/time.h>
> @@ -62,40 +63,42 @@ static int mapLogLevel(int level)
> }
> }
>
> +static void log_level_to_str(const int level, char* outbuf)
> +{
> + switch (level) {
> + case DEBUGLVL:
> + strcpy(outbuf, "DEBUG");
> + break;
> + case INFO:
> + strcpy(outbuf, "INFO");
> + break;
> + case WARNING:
> + strcpy(outbuf, "WARN");
> + break;
> + case ERROR:
> + strcpy(outbuf, "ERR");
> + break;
> + case CRITICAL:
> + strcpy(outbuf, "CRIT");
> + break;
> + default:
> + strcpy(outbuf, "(UNKNWN)");
> + break;
> + }
> +}
> +
> static void printLogHeader(int level, const char *tag, FILE *outfile) {
> struct timeval current_time;
> struct tm *t;
> int msecs;
> + char level_name[10];
>
> gettimeofday(&current_time, NULL);
> t = gmtime(&current_time.tv_sec);
> msecs = current_time.tv_usec / 1000;
> - switch (level) {
> - case DEBUGLVL:
> - fprintf (outfile, "%02d:%02d:%02d,%03d DEBUG %s: ", t->tm_hour,
> - t->tm_min, t->tm_sec, msecs, tag);
> - break;
> -
> - case INFO:
> - fprintf (outfile, "%02d:%02d:%02d,%03d INFO %s: ", t->tm_hour,
> - t->tm_min, t->tm_sec, msecs, tag);
> - break;
> -
> - case WARNING:
> - fprintf (outfile, "%02d:%02d:%02d,%03d WARN %s: ", t->tm_hour,
> - t->tm_min, t->tm_sec, msecs, tag);
> - break;
> -
> - case ERROR:
> - fprintf (outfile, "%02d:%02d:%02d,%03d ERR %s: ", t->tm_hour,
> - t->tm_min, t->tm_sec, msecs, tag);
> - break;
> -
> - case CRITICAL:
> - fprintf (outfile, "%02d:%02d:%02d,%03d CRIT %s: ", t->tm_hour,
> - t->tm_min, t->tm_sec, msecs, tag);
> - break;
> - }
> + log_level_to_str(level, level_name);
> + fprintf(outfile, "%02d:%02d:%02d,%03d %s %s: ", t->tm_hour,
> + t->tm_min, t->tm_sec, msecs, level_name, tag);
> }
>
> static void printLogMessage(int level, const char *tag, FILE *outfile, const char *s, va_list ap)

Steffen

Linux on System z Development

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

> const char log_level_to_str = { ..., ... }
> and keep it in sync with constant definitions of DEBUGLEVEL et al.?

Yeah, something like that does seem more clean to me, too. The log
levels haven't changed in years and aren't likely to, so keeping things
in sync should be really easy.

- Chris

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

From: Ales Kozumplik <akozumpl@redhat.com>

---
pyanaconda/isys/log.c | 39 +++++++++++++--------------------------
pyanaconda/isys/log.h | 1 +
2 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/pyanaconda/isys/log.c b/pyanaconda/isys/log.c
index 96da57e..0421684 100644
--- a/pyanaconda/isys/log.c
+++ b/pyanaconda/isys/log.c
@@ -27,6 +27,7 @@
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
+#include <string.h>
#include <time.h>
#include <unistd.h>
#include <sys/time.h>
@@ -62,40 +63,26 @@ static int mapLogLevel(int level)
}
}

+const char *log_level_to_str[LOGLEVEL_MAX + 1] = {
+ [DEBUGLVL] = "DEBUG",
+ [INFO] = "INFO",
+ [WARNING] = "WARN",
+ [ERROR] = "ERR",
+ [CRITICAL] = "CRIT"
+};
+
static void printLogHeader(int level, const char *tag, FILE *outfile) {
struct timeval current_time;
struct tm *t;
int msecs;
+ const char *level_name;

gettimeofday(&current_time, NULL);
t = gmtime(&current_time.tv_sec);
msecs = current_time.tv_usec / 1000;
- switch (level) {
- case DEBUGLVL:
- fprintf (outfile, "%02d:%02d:%02d,%03d DEBUG %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case INFO:
- fprintf (outfile, "%02d:%02d:%02d,%03d INFO %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case WARNING:
- fprintf (outfile, "%02d:%02d:%02d,%03d WARN %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case ERROR:
- fprintf (outfile, "%02d:%02d:%02d,%03d ERR %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case CRITICAL:
- fprintf (outfile, "%02d:%02d:%02d,%03d CRIT %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
- }
+ level_name = log_level_to_str[level];
+ fprintf(outfile, "%02d:%02d:%02d,%03d %s %s: ", t->tm_hour,
+ t->tm_min, t->tm_sec, msecs, level_name, tag);
}

static void printLogMessage(int level, const char *tag, FILE *outfile, const char *s, va_list ap)
diff --git a/pyanaconda/isys/log.h b/pyanaconda/isys/log.h
index 88d0010..910fd9a 100644
--- a/pyanaconda/isys/log.h
+++ b/pyanaconda/isys/log.h
@@ -28,6 +28,7 @@
#define WARNING 30
#define ERROR 40
#define CRITICAL 50
+#define LOGLEVEL_MAX CRITICAL

enum logger_t {
MAIN_LOG = 1,
--
1.7.1.1

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

---
pyanaconda/isys/log.c | 39 +++++++++++++--------------------------
pyanaconda/isys/log.h | 1 +
2 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/pyanaconda/isys/log.c b/pyanaconda/isys/log.c
index 96da57e..0421684 100644
--- a/pyanaconda/isys/log.c
+++ b/pyanaconda/isys/log.c
@@ -27,6 +27,7 @@
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
+#include <string.h>
#include <time.h>
#include <unistd.h>
#include <sys/time.h>
@@ -62,40 +63,26 @@ static int mapLogLevel(int level)
}
}

+const char *log_level_to_str[LOGLEVEL_MAX + 1] = {
+ [DEBUGLVL] = "DEBUG",
+ [INFO] = "INFO",
+ [WARNING] = "WARN",
+ [ERROR] = "ERR",
+ [CRITICAL] = "CRIT"
+};
+
static void printLogHeader(int level, const char *tag, FILE *outfile) {
struct timeval current_time;
struct tm *t;
int msecs;
+ const char *level_name;

gettimeofday(&current_time, NULL);
t = gmtime(&current_time.tv_sec);
msecs = current_time.tv_usec / 1000;
- switch (level) {
- case DEBUGLVL:
- fprintf (outfile, "%02d:%02d:%02d,%03d DEBUG %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case INFO:
- fprintf (outfile, "%02d:%02d:%02d,%03d INFO %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case WARNING:
- fprintf (outfile, "%02d:%02d:%02d,%03d WARN %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case ERROR:
- fprintf (outfile, "%02d:%02d:%02d,%03d ERR %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
-
- case CRITICAL:
- fprintf (outfile, "%02d:%02d:%02d,%03d CRIT %s: ", t->tm_hour,
- t->tm_min, t->tm_sec, msecs, tag);
- break;
- }
+ level_name = log_level_to_str[level];
+ fprintf(outfile, "%02d:%02d:%02d,%03d %s %s: ", t->tm_hour,
+ t->tm_min, t->tm_sec, msecs, level_name, tag);
}

static void printLogMessage(int level, const char *tag, FILE *outfile, const char *s, va_list ap)
diff --git a/pyanaconda/isys/log.h b/pyanaconda/isys/log.h
index 88d0010..910fd9a 100644
--- a/pyanaconda/isys/log.h
+++ b/pyanaconda/isys/log.h
@@ -28,6 +28,7 @@
#define WARNING 30
#define ERROR 40
#define CRITICAL 50
+#define LOGLEVEL_MAX CRITICAL

enum logger_t {
MAIN_LOG = 1,
--
1.7.1.1

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

> +const char *log_level_to_str[LOGLEVEL_MAX + 1] = {

This isn't needed, just keep the brackets empty and the compiler will do its job to allocate only the smallest array neccessary.

--
Martin Sivák
msivak@redhat.com
Red Hat Czech
Anaconda team / Brno, CZ

----- "Ales Kozumplik" <akozumpl@redhat.com> wrote:

> ---
> pyanaconda/isys/log.c | 39 +++++++++++++--------------------------
> pyanaconda/isys/log.h | 1 +
> 2 files changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/pyanaconda/isys/log.c b/pyanaconda/isys/log.c
> index 96da57e..0421684 100644
> --- a/pyanaconda/isys/log.c
> +++ b/pyanaconda/isys/log.c
> @@ -27,6 +27,7 @@
> #include <stdarg.h>
> #include <stdio.h>
> #include <stdlib.h>
> +#include <string.h>
> #include <time.h>
> #include <unistd.h>
> #include <sys/time.h>
> @@ -62,40 +63,26 @@ static int mapLogLevel(int level)
> }
> }
>
> +const char *log_level_to_str[LOGLEVEL_MAX + 1] = {
> + [DEBUGLVL] = "DEBUG",
> + [INFO] = "INFO",
> + [WARNING] = "WARN",
> + [ERROR] = "ERR",
> + [CRITICAL] = "CRIT"
> +};
> +
> static void printLogHeader(int level, const char *tag, FILE *outfile)
> {
> struct timeval current_time;
> struct tm *t;
> int msecs;
> + const char *level_name;
>
> gettimeofday(&current_time, NULL);
> t = gmtime(&current_time.tv_sec);
> msecs = current_time.tv_usec / 1000;
> - switch (level) {
> - case DEBUGLVL:
> - fprintf (outfile, "%02d:%02d:%02d,%03d DEBUG %s: ",
> t->tm_hour,
> - t->tm_min, t->tm_sec, msecs, tag);
> - break;
> -
> - case INFO:
> - fprintf (outfile, "%02d:%02d:%02d,%03d INFO %s: ",
> t->tm_hour,
> - t->tm_min, t->tm_sec, msecs, tag);
> - break;
> -
> - case WARNING:
> - fprintf (outfile, "%02d:%02d:%02d,%03d WARN %s: ",
> t->tm_hour,
> - t->tm_min, t->tm_sec, msecs, tag);
> - break;
> -
> - case ERROR:
> - fprintf (outfile, "%02d:%02d:%02d,%03d ERR %s: ",
> t->tm_hour,
> - t->tm_min, t->tm_sec, msecs, tag);
> - break;
> -
> - case CRITICAL:
> - fprintf (outfile, "%02d:%02d:%02d,%03d CRIT %s: ",
> t->tm_hour,
> - t->tm_min, t->tm_sec, msecs, tag);
> - break;
> - }
> + level_name = log_level_to_str[level];
> + fprintf(outfile, "%02d:%02d:%02d,%03d %s %s: ", t->tm_hour,
> + t->tm_min, t->tm_sec, msecs, level_name, tag);
> }
>
> static void printLogMessage(int level, const char *tag, FILE
> *outfile, const char *s, va_list ap)
> diff --git a/pyanaconda/isys/log.h b/pyanaconda/isys/log.h
> index 88d0010..910fd9a 100644
> --- a/pyanaconda/isys/log.h
> +++ b/pyanaconda/isys/log.h
> @@ -28,6 +28,7 @@
> #define WARNING 30
> #define ERROR 40
> #define CRITICAL 50
> +#define LOGLEVEL_MAX CRITICAL
>
> enum logger_t {
> MAIN_LOG = 1,
> --
> 1.7.1.1
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@redhat.com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/16/2010 02:08 AM, Ales Kozumplik wrote:

> +const char *log_level_to_str[LOGLEVEL_MAX + 1] = {
> + [DEBUGLVL] = "DEBUG",
> + [INFO] = "INFO",
> + [WARNING] = "WARN",
> + [ERROR] = "ERR",
> + [CRITICAL] = "CRIT"
> +};
> +

Neat! That's the first time I've seen that. It does seem a bit fragile
though -- pass it a wrong index and it segfaults.

> + level_name = log_level_to_str[level];
> + fprintf(outfile, "%02d:%02d:%02d,%03d %s %s: ", t->tm_hour,
> + t->tm_min, t->tm_sec, msecs, level_name, tag);
> }

level needs a check on it to make sure it is valid. I tried out a
snippet of code to test this and unless you pass it one of the
initializers it will segfault.

- --
Brian C. Lane <bcl@redhat.com>
Red Hat / Port Orchard, WA
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Remember Lexington Green!

iQEVAwUBTGlzihF+jBaO/jp/AQKWagf/Y2yBnmlOdLZwuK9KeQv1w/0vGRql2hSX
iiy9XxxXDB3XygxfLBlEZQcjr7UKhGVhGK9HRuchXeO6nSJAdv 09igUKSs/bszV4
39AFRlPlLTuUsTnDSwX34Zpla48LsmG8E6L+zRhOPB16LkcwWN M71V50qWDb21ki
4UItaFLIZ3yUbulE4K4VN0jSUcttp6UmH7FUerpIW3m9yFBdra ZUzbuZgWYFSTXP
Qt76DBvtfMel3y27Ulr+xoTQOukvEao9D9JZlcpz2BURF7nQi9 twYpN1A2Sm2AiS
7KGt1BEQm//E56TtIDRxF6iAL2uymWr8Kkzs2rDYmtcbCpKzbl/NMg==
=wp2n
-----END PGP SIGNATURE-----

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

If you're going to do this

> +const char *log_level_to_str[LOGLEVEL_MAX + 1] = {
> + [DEBUGLVL] = "DEBUG",
> + [INFO] = "INFO",
> + [WARNING] = "WARN",
> + [ERROR] = "ERR",
> + [CRITICAL] = "CRIT"
> +};

Please fix this:

> #define WARNING 30
> #define ERROR 40
> #define CRITICAL 50
> +#define LOGLEVEL_MAX CRITICAL

To be a real enum or at least not have gaps of 10 in it. That array doesn't
need 50 elements.

--
Peter

Gravity is a habit that is hard to shake off.
-- Pratchett

01234567890123456789012345678901234567890123456789 012345678901234567890123456789

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

On 08/16/2010 07:21 PM, Brian C. Lane wrote:

+ level_name = log_level_to_str[level];
+ fprintf(outfile, "%02d:%02d:%02d,%03d %s %s: ", t->tm_hour,
+ t->tm_min, t->tm_sec, msecs, level_name, tag);
}


level needs a check on it to make sure it is valid. I tried out a
snippet of code to test this and unless you pass it one of the
initializers it will segfault.


This should be safer now when the loglevel is an enum type.

Ales

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list