[ofa-general] Re: [PATCH 1/6] complib/nodenamemap: add generic parse_node_map() function

[Ira Weiny]

Hey Sasha,

On Thu, 27 Mar 2008 00:27:12 +0000
Sasha Khapyorsky <sashak at voltaire.com> wrote:

> This parser is able to parse text file with format similar to nodenamemap
> file:
> 
> 	guid <string>  # comment
> 	....

I like the idea of using this same format.

> 
> It will be used later for unification parsing of root nodes and compute
> nodes guid list files.
> 
> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> ---
>  opensm/complib/cl_nodenamemap.c         |   45 +++++++++++++++++++++++++++++++
>  opensm/complib/libosmcomp.map           |    1 +
>  opensm/include/complib/cl_nodenamemap.h |    3 ++
>  3 files changed, 49 insertions(+), 0 deletions(-)
> 
> diff --git a/opensm/complib/cl_nodenamemap.c b/opensm/complib/cl_nodenamemap.c
> index af5da39..696d455 100644
> --- a/opensm/complib/cl_nodenamemap.c
> +++ b/opensm/complib/cl_nodenamemap.c
> @@ -160,3 +160,48 @@ clean_nodedesc(char *nodedesc)
>  
>  	return (nodedesc);
>  }
> +
> +int parse_node_map(const char *file_name,
> +		   int (*create)(void *, uint64_t, char *), void *cxt)

Why do you need this new way of parsing the file format?  I think passing this
callback makes the code more complicated that it needs to be.

Is there a reason not to use the nn_map_t as the type for root_nodes_list in
the updn_t struct?  We could rename nn_map_t to be more generic if necessary.

I worry about maintaining 2 functions which parse the same file format in the
same c file.

Ira

> +{
> +	char line[256];
> +	FILE *f;
> +
> +	if (!(f = fopen(file_name, "r")))
> +		return -1;
> +
> +	while (fgets(line, sizeof(line),f)) {
> +		uint64_t guid;
> +		char *p, *e;
> +
> +		p = line;
> +		while (isspace(*p))
> +			p++;
> +		if (*p == '\0' || *p == '\n' || *p == '#')
> +			continue;
> +
> +		guid = strtoull(p, &e, 0);
> +		if (e == p || !isspace(*e)) {
> +			fclose(f);
> +			return -1;
> +		}
> +
> +		p = e;
> +		if (*e)
> +			e++;
> +		while (isspace(*p))
> +			p++;
> +
> +		e = strpbrk(p, "# \t\n");
> +		if (e)
> +			*e = '\0';
> +
> +		if (create(cxt, guid, p)) {
> +			fclose(f);
> +			return -1;
> +		}
> +	}
> +
> +	fclose(f);
> +	return 0;
> +}
> diff --git a/opensm/complib/libosmcomp.map b/opensm/complib/libosmcomp.map
> index d0e107e..788eb2a 100644
> --- a/opensm/complib/libosmcomp.map
> +++ b/opensm/complib/libosmcomp.map
> @@ -147,6 +147,7 @@ OSMCOMP_2.3 {
>  		ib_wc_status_str;
>  		open_node_name_map;
>  		close_node_name_map;
> +		parse_node_map;
>  		remap_node_name;
>  		clean_nodedesc;
>  	local: *;
> diff --git a/opensm/include/complib/cl_nodenamemap.h b/opensm/include/complib/cl_nodenamemap.h
> index b20ca4e..5f4a026 100644
> --- a/opensm/include/complib/cl_nodenamemap.h
> +++ b/opensm/include/complib/cl_nodenamemap.h
> @@ -1,4 +1,5 @@
>  /*
> + * Copyright (c) 2008 Voltaire, Inc. All rights reserved.
>   * Copyright (c) 2007 Lawrence Livermore National Lab
>   *
>   * This software is available to you under a choice of one of two
> @@ -60,5 +61,7 @@ void  close_node_name_map(nn_map_t *map);
>  char *remap_node_name(nn_map_t *map, uint64_t target_guid,
>  			char *nodedesc);
>  	/* NOTE: parameter "nodedesc" may be modified here. */
> +int parse_node_map(const char *file_name,
> +		   int (*create)(void *, uint64_t, char *), void *cxt);
>  
>  #endif				/* _CL_NODE_NAME_MAP_H_ */
> -- 
> 1.5.4.1.122.gaa8d