Gilles Carry wrote:
> This library provides a set of function to help the saving of data as XML
> formatted files.
> Note: this is a lightweight library as it does not check if the document is 
> well formed or grammaticallt correct.
> It must be considered as a commodity that helps tag indentation and avoids 
> user to toss with '<' and '>'.
> To embbed it into executables, run:
> XML_LIB=1 make
> 
> Also, it adds heading tags with data such as timestamp, system information, 
> test conditions... This to facilitate post processing. (eg. further 
> comparisons of different testruns, formatting for plotting...)
> 
> This patch does not alter the LTP/RT traditional stats dump.
> A new global command line option is used: -x <id>
> 
> Compilation is conditional to LIB_XML.


Implementation and separation look good.  I have a couple questions 
below on a couple details.  I mentioned a few coding standards nitpics 
while I was at it so you see them early on and they don't delay things 
in the future, but they clearly aren't a priority now.

--
Darren

> 
> Signed-off-by: Gilles Carry <[EMAIL PROTECTED]>
> ---
>  testcases/realtime/config.mk        |    6 +
>  testcases/realtime/include/libxml.h |  112 +++++++++++
>  testcases/realtime/lib/librttest.c  |   35 ++++-
>  testcases/realtime/lib/libxml.c     |  350 
> +++++++++++++++++++++++++++++++++++
>  4 files changed, 502 insertions(+), 1 deletions(-)
>  create mode 100644 testcases/realtime/include/libxml.h
>  create mode 100644 testcases/realtime/lib/libxml.c
> 
> diff --git a/testcases/realtime/config.mk b/testcases/realtime/config.mk
> index 19ccddc..790aa17 100644
> --- a/testcases/realtime/config.mk
> +++ b/testcases/realtime/config.mk
> @@ -18,8 +18,14 @@ endif
>  #
>  CPPFLAGS += -I$(srcdir)/include -D_GNU_SOURCE
>  CFLAGS   += -Wall
> +ifdef LIB_XML
> +LDLIBS   += $(srcdir)/lib/libxml.o
> +CFLAGS   += -DLIB_XML
> +endif
> +
>  LDLIBS   += $(srcdir)/lib/libjvmsim.o \
>          $(srcdir)/lib/librttest.o \
>          $(srcdir)/lib/libstats.o \
>          -lpthread -lrt -lm
> 
> +CFLAGS   += -m64 -g

This appears to be a superfluous change, or at least unrelated to libxml...

> diff --git a/testcases/realtime/include/libxml.h 
> b/testcases/realtime/include/libxml.h
> new file mode 100644
> index 0000000..1291ee8
> --- /dev/null
> +++ b/testcases/realtime/include/libxml.h
> @@ -0,0 +1,112 @@
> +/******************************************************************************
> + *
> + *   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
> + *   the Free Software Foundation; either version 2 of the License, or
> + *   (at your option) any later version.
> + *
> + *   This program is distributed in the hope that it will be useful,
> + *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> + *   the GNU General Public License for more details.
> + *
> + *   You should have received a copy of the GNU General Public License
> + *   along with this program;  if not, write to the Free Software
> + *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + * NAME
> + *      libxml.c

You have NAME in here twice, the one above is incorrect.

> + *
> + * DESCRIPTION
> + *      Lightweight xml functions
> + *
> + * USAGE:
> + *      To be linked with testcases
> + *
> + * AUTHOR
> + *        Gilles Carry <[EMAIL PROTECTED]>

Minor nit, ^ alignment is off (only mentioned because I had other 
comments ;-)

> + *
> + * HISTORY
> + *      2008-Oct-20: Initial version by Gilles Carry
> + *
> + * NAME
> + *      libxml.h
> + *
> + * TODO:
> + *
> + 
> *****************************************************************************/
> +
> +#ifndef LIBXML_H
> +#define LIBXML_H
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <unistd.h>
> +
> +typedef struct xml_stream {
> +     FILE * fd;
> +     int indent_level;
> +} xml_stream_t;
> +
> +extern char *test_name;
> +extern char *xml_user_free_id;
> +extern int xml_dump;

Hrm... seems to me these should be stream specific... reading on to 
understand better...

> +
> +/*
> + * Write a start tag into an xml stream:
> + * <tagname>
> + *
> + * xs: xml stream to write to
> + * tagname: xml markup name
> + */
> +void xml_start_tag (xml_stream_t *xs, char *tagname);
> +
> +/*
> + * Write a end tag into an xml stream:

             ^ an

> + * </tagname>
> + *
> + * xs: xml stream to write to
> + * tagname: xml markup name
> + */
> +void xml_end_tag (xml_stream_t *xs, char *tagname);
> +

Hrm.. so you stated this is intended to be simple, and that's fine for 
the first round.  In the future, should this get used more, I wonder if
it might not make sense to return some kind of handle when a tag is 
started, then close the tag with that handle.  That would allow the 
library to ensure the spelling is correct, etc.  Minimize user error.

> +/*
> + * Write an xml entry into an xml stream:
> + * <tagname>string</tagname>
> + * 
> + * xs: xml stream to write to
> + * tagname: xml markup name
> + * fmt, ...: printf style formatting
> + */
> +void xml_entry (xml_stream_t *xs, char *tagname, char *fmt, ...);
> +
> +/*
> + * Write an xml empty tag:
> + * <tagname/>
> + * 
> + * xs: xml stream to write to
> + * tagname: xml tag name
> + */
> +void xml_empty_tag (xml_stream_t *xs, char *tagname);
> +
> +/*
> + * Create XML stream file and initialize headers.
> + * testname: name of the test
> + * root_tag: tag of root element
> + * title: title of the XML stream
> + *
> + * return: xml_stream_t pointer to be reused by subsequent xml_... calls.
> + */
> +xml_stream_t * xml_stream_init(char *testname, char *root_tag, char *title);
> +
> +/*
> + * Close XML stream.
> + * root_tag: tag of root element
> + *
> + */
> +void xml_stream_close(xml_stream_t *xs, char *root_tag);
> +
> +
> +#endif /* LIBXML_H */
> diff --git a/testcases/realtime/lib/librttest.c 
> b/testcases/realtime/lib/librttest.c
> index cd76b63..f72fff6 100644
> --- a/testcases/realtime/lib/librttest.c
> +++ b/testcases/realtime/lib/librttest.c
> @@ -41,6 +41,9 @@
>   
> *****************************************************************************/
> 
>  #include <librttest.h>
> +#ifdef LIB_XML
> +#include <libxml.h>
> +#endif
>  #include <libstats.h>
> 
>  #include <stdio.h>
> @@ -79,6 +82,10 @@ void rt_help(void)
>       printf("  -p(0,1)       0:don't use pi mutexes, 1:use pi mutexes\n");
>       printf("  -v[0-4]       0:no debug, 1:DBG_ERR, 2:DBG_WARN, 3:DBG_INFO, 
> 4:DBG_DEBUG\n");
>       printf("  -s            Enable saving stats data (default disabled)\n");
> +#ifdef LIB_XML
> +     printf("  -x <user_free_id>     Enable saving of outputs into xml file 
> (default disabled)\n");
> +     printf("                user_free_id (mandatory arg) string inserted 
> into xml dump\n");
> +#endif
>       printf("  -c            Set pass criteria\n");
>  }
> 
> @@ -90,7 +97,11 @@ int rt_init(const char *options, int (*parse_arg)(int 
> option, char *value), int
>       opterr = 0;
>       char *all_options;
> 
> -     if (asprintf(&all_options, ":b:p:v:sc:%s", options) == -1) {
> +     if (asprintf(&all_options, ":b:p:v:sc:"
> +#ifdef LIB_XML
> +     "x:"
> +#endif
> +     "%s", options) == -1) {
>               fprintf(stderr, "Failed to allocate string for option 
> string\n");
>               exit(1);
>       }
> @@ -108,6 +119,20 @@ int rt_init(const char *options, int (*parse_arg)(int 
> option, char *value), int
>                       exit(1);
>               }
>       }
> +
> +     /* Check for duplicate options in optstring */
> +     for (i=0; i<strlen(all_options); i++) {
> +             char opt = all_options[i];
> +
> +             if (opt == ':')
> +                     continue;
> +
> +             /* Search ahead */
> +             if (strchr(&all_options[i+1], opt)) {
> +                     fprintf(stderr, "Programmer error -- argument -%c 
> already used at least twice\n", opt);
> +                     exit(1);
> +             }
> +     }

The above doesn't appear related to libxml...

>       
>       while ((c = getopt(argc, argv, all_options)) != -1) {
>               switch (c) {
> @@ -126,6 +151,14 @@ int rt_init(const char *options, int (*parse_arg)(int 
> option, char *value), int
>               case 's':
>                       save_stats = 1;
>                       break;
> +#ifdef LIB_XML
> +             case 'x':
> +                     xml_dump = 1;
> +                     xml_user_free_id = optarg;
> +                     if (!strcmp("", xml_user_free_id))
> +                             return -1;
> +                     break;
> +#endif
>               case ':':
>                       fprintf(stderr, "option -%c: missing arg\n", optopt);
>                       parse_arg('h', optarg); /* Just to display usage */
> diff --git a/testcases/realtime/lib/libxml.c b/testcases/realtime/lib/libxml.c
> new file mode 100644
> index 0000000..6e1292f
> --- /dev/null
> +++ b/testcases/realtime/lib/libxml.c
> @@ -0,0 +1,350 @@
> +/******************************************************************************
> + *
> + *   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
> + *   the Free Software Foundation; either version 2 of the License, or
> + *   (at your option) any later version.
> + *
> + *   This program is distributed in the hope that it will be useful,
> + *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> + *   the GNU General Public License for more details.
> + *
> + *   You should have received a copy of the GNU General Public License
> + *   along with this program;  if not, write to the Free Software
> + *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + * NAME
> + *      libxml.c
> + *
> + * DESCRIPTION
> + *      Lightweight xml functions
> + *
> + *
> + * USAGE:
> + *      To be linked with testcases
> + *
> + * AUTHOR
> + *        Gilles Carry <[EMAIL PROTECTED]>
> + *
> + * HISTORY
> + *      2008-Oct-20: Initial version by Gilles Carry
> + *
> + * TODO:
> + *
> + 
> *****************************************************************************/
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <dirent.h>
> +#include <ctype.h>
> +#include <sys/utsname.h>
> +#include <stdarg.h>
> +#include <libxml.h>
> +#include <librttest.h>
> +
> +char *xml_user_free_id = "";
> +int xml_dump = 0;

Why can't these part of the xml_stream structure?

> +
> +
> +/*
> + * Fetch information across /sys to provide the cpu topology
> + * of the system.
> + */
> +static void xml_get_cpu_topology(xml_stream_t * xs)
> +{
> +     DIR *directory_parent, *directory_node;
> +     struct dirent *de,*dn;
> +     char directory_path[255];
> +     char *sys_dir;
> +     char *ts_fname="thread_siblings_list";
> +     char buf[200];
> +
> +     xml_start_tag(xs, "sys");
> +     xml_start_tag(xs, "devices");
> +     xml_start_tag(xs, "system");
> +
> +     xml_start_tag(xs, "node");
> +
> +     sys_dir = "/sys/devices/system/node";
> +     directory_parent = opendir(sys_dir);
> +     if (!directory_parent)  {
> +             xml_empty_tag(xs, "not-numa");
> +     } else {
> +             while ((de = readdir(directory_parent)) != NULL) {
> +                     if (strncmp(de->d_name, "node", 4))
> +                             continue;
> +
> +                     /* Check if string matches /node[0-9]/ */
> +                     if (!isdigit(de->d_name[4]))
> +                             continue;
> +
> +                     xml_start_tag(xs, de->d_name); /* <nodeX> */
> +
> +                     sprintf(directory_path,"%s/%s",sys_dir, de->d_name);

coding standards                               ^ need a space

> +                     directory_node = opendir(directory_path);
> +                     if (!directory_parent)  {
> +                             fprintf(stderr, "Unable to open dir %s: %s\n", 
> directory_path, strerror(errno));
> +                             continue;
> +                     }
> +                     while ((dn = readdir(directory_node)) != NULL) {
> +                             if (strncmp(dn->d_name, "cpu", 3))
> +                                     continue;
> +
> +                             if (!isdigit(dn->d_name[3]))
> +                                     continue;
> +
> +                             xml_empty_tag(xs, dn->d_name); /* <cpuX/> */
> +                     }
> +                     xml_end_tag(xs, de->d_name); /* </nodeX> */
> +                     closedir(directory_node);
> +             }
> +             closedir(directory_parent);
> +     }
> +     xml_end_tag(xs, "node");
> +
> +
> +     xml_start_tag(xs, "cpu");
> +     sys_dir = "/sys/devices/system/cpu";
> +     directory_parent = opendir(sys_dir);
> +     if (!directory_parent) {
> +             fprintf(stderr, "Unable to open dir %s: %s\n", sys_dir, 
> strerror(errno));
> +             exit(EXIT_FAILURE);
> +     }
> +     while ((de = readdir(directory_parent)) != NULL) {
> +
> +             if (strncmp(de->d_name, "cpu", 3))
> +                     continue;
> +
> +             /* Check if string matches /node[0-9]/ */
> +             if (!isdigit(de->d_name[3]))
> +                     continue;
> +
> +             sprintf(directory_path,"%s/%s/topology",sys_dir, de->d_name);
> +             directory_node = opendir(directory_path);
> +             if (!directory_node) {
> +                     /* Cpu probably offline */
> +                     continue;
> +             }
> +             xml_start_tag(xs, de->d_name);
> +
> +             xml_start_tag(xs, "topology");
> +             while ((dn = readdir(directory_node)) != NULL) {
> +                     char *fpath;
> +                     FILE *fd_sib;
> +                     size_t rd;
> +                     char *cr;
> +
> +
> +                     if (strncmp(dn->d_name, ts_fname, strlen(ts_fname)))
> +                             continue;
> +
> +                     asprintf(&fpath, "%s/%s", directory_path, dn->d_name);
> +                     fd_sib = fopen(fpath, "r");
> +                     if (fd_sib == NULL) {
> +                             fprintf(stderr, "Unable to open file %s: %s\n", 
> fpath, strerror(errno));
> +                             continue;
> +                     }
> +
> +
> +                     rd = fread(buf, 1, sizeof(buf), fd_sib);
> +                     if (rd == sizeof(buf))
> +                             fprintf(stderr, "%s:fread: probable 
> overflow\n", __FILE__);
> +                     fclose(fd_sib);
> +                     free(fpath);
> +
> +                     /*
> +                      * chop off \n.
> +                      * Note: We consider that only one \n is present
> +                      * at the end of the file.
> +                      */
> +                     cr = strchr(buf, '\n');
> +                     if(cr)
> +                             *cr = '\0';
> +
> +                     xml_entry(xs, ts_fname, buf);
> +             }
> +             xml_end_tag(xs, "topology");
> +             xml_end_tag(xs, de->d_name);
> +             closedir(directory_node);
> +     }
> +     closedir(directory_parent);
> +     xml_end_tag(xs, "cpu");
> +
> +     xml_end_tag(xs, "system");
> +     xml_end_tag(xs, "devices");
> +     xml_end_tag(xs, "sys");
> +}
> +
> +/*
> + * Write a start tag into an xml stream:
> + * <tagname>
> + *
> + * xs: xml stream to write to
> + * tagname: xml markup name
> + */
> +void xml_start_tag (xml_stream_t *xs, char *tagname)

                      ^ extra space

> +{
> +     int i;
> +
> +     for(i = 0; i < xs->indent_level; i++)

            ^ space

> +             fprintf(xs->fd,"\t");
> +
> +     fprintf(xs->fd, "<%s>\n", tagname);
> +
> +     xs->indent_level++;
> +}
> +
> +/*
> + * Write a end tag into an xml stream:
> + * </tagname>
> + *
> + * xs: xml stream to write to
> + * tagname: xml markup name
> + */
> +void xml_end_tag (xml_stream_t *xs, char *tagname)

                      ^ extra space

> +{
> +     int i;
> +
> +     xs->indent_level--;
> +
> +     for(i = 0; i < xs->indent_level; i++)

             ^ space

> +             fprintf(xs->fd,"\t");
> +
> +     fprintf(xs->fd, "</%s>\n", tagname);
> +
> +}
> +
> +/*
> + * Write an xml entry into an xml stream:
> + * <tagname>string</tagname>
> + * 
> + * xs: xml stream to write to
> + * tagname: xml markup name
> + * fmt, ...: printf style formatting
> + */
> +void xml_entry (xml_stream_t *xs, char *tagname, char *fmt, ...)

                  ^ extra space

> +{
> +     va_list ap;
> +     int i;
> +
> +     for(i = 0; i < xs->indent_level; i++)

            ^ space

> +             fprintf(xs->fd,"\t");
> +
> +     va_start(ap, fmt);
> +     fprintf(xs->fd, "<%s>", tagname);
> +     vfprintf(xs->fd, fmt, ap);
> +     fprintf(xs->fd, "</%s>\n", tagname);
> +     va_end(ap);
> +}
> +
> +/*
> + * Write an xml empty tag:
> + * <tagname/>
> + * 
> + * xs: xml stream to write to
> + * tagname: xml tag name
> + */
> +void xml_empty_tag (xml_stream_t *xs, char *tagname)

                      ^ extra space

> +{
> +     int i;
> +
> +     for(i = 0; i < xs->indent_level; i++)

            ^ space

> +             fprintf(xs->fd,"\t");
> +
> +     fprintf(xs->fd, "<%s/>\n", tagname);
> +
> +}
> +
> +/*
> + * Create XML stream file and initialize headers.
> + * testname: name of the test
> + * root_tag: tag of root element
> + * title: title of the XML stream
> + *
> + * return: xml_stream_t pointer to be reused by subsequent xml_... calls.
> + */
> +xml_stream_t * xml_stream_init(char *testname, char *root_tag, char *title)
> +{
> +     FILE *fd;
> +     struct utsname u;
> +     char *xmlfile;
> +     xml_stream_t *xs;
> +     time_t t;
> +     struct tm *tmp;
> +     char start_time[20];
> +
> +     xs = malloc (sizeof(xml_stream_t));
> +     if (xs == NULL) {
> +             perror("xml_stream_init:malloc");
> +             return NULL;
> +     }
> +
> +     t = time(NULL);
> +     tmp = localtime(&t);
> +     if (tmp == NULL) {
> +             perror("localtime");
> +             exit(EXIT_FAILURE);
> +     }
> +
> +     if (strftime(start_time, sizeof(start_time), "%Y-%m-%d.%H-%M-%S", tmp) 
> == 0) {
> +             fprintf(stderr, "strftime returned 0");
> +             exit(EXIT_FAILURE);
> +     }
> +
> +     xs->indent_level = 0;
> +
> +
> +     if (-1 == asprintf(&xmlfile, "%s.%s.xml", testname, start_time)) {
> +             fprintf(stderr, "xml_stream_init: Failed to allocate string for 
> data filename\n");
> +             return NULL;
> +     }
> +
> +     /* generate the data file */
> +     if (!(fd = fopen(xmlfile, "w")))
> +             return NULL;
> +     
> +     xs->fd = fd;
> +
> +     uname(&u);
> +
> +     fprintf(xs->fd, "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
> +
> +     xml_start_tag(xs, root_tag);
> +
> +     /* Headers */
> +     xml_entry(xs, "title", title);
> +     xml_entry(xs, "testname", testname);
> +     xml_entry(xs, "start-time", start_time);
> +     xml_start_tag(xs, "systeminfo");
> +     xml_start_tag(xs, "uname");
> +     xml_entry(xs, "sysname", u.sysname);
> +     xml_entry(xs, "nodename", u.nodename);
> +     xml_entry(xs, "release", u.release);
> +     xml_entry(xs, "version", u.version);
> +     xml_entry(xs, "machine", u.machine);
> +     xml_end_tag(xs, "uname");
> +     xml_get_cpu_topology(xs);
> +     xml_entry(xs, "user-free-id", xml_user_free_id);
> +     xml_entry(xs, "online-cpus", "%ld", sysconf(_SC_NPROCESSORS_ONLN));
> +     xml_end_tag(xs, "systeminfo");
> +     xml_entry(xs, "filename", xmlfile);
> +
> +     return xs;
> +}
> +
> +/*
> + * Close XML stream.
> + * root_tag: tag of root element
> + *
> + */
> +void xml_stream_close(xml_stream_t *xs, char *root_tag)
> +{
> +     xml_end_tag(xs, root_tag);
> +     fclose(xs->fd);
> +     free(xs);
> +     return;
> +}
> +


-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to