On Wed, Jan 08, 2014 at 01:31:44PM +0100, Arturo Borrero Gonzalez wrote:
> On 8 January 2014 00:29, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> >
> > You should pass a callback function instead, eg.
> >
> > static int nft_chain_json_parse(struct nft_chain *c, const char *json,
> > struct nft_parse_err *err,
> > json_t *(*jsonbuilder)(const void *input_data,
> > const char *treename,
> > struct nft_parse_err *e))
>
> Ok.
>
> > But I don't understand yet what you save (in terms of lines of code)
> > by using this aproach.
>
> I avoid doing something like:
>
> nft_*_parse() {
> switch (type)
> if XML return xml_parse()
> if JSON return json_parse()
> }
>
> nft_*_parse_file() {
> switch (type)
> if XML return xml_parse_file()
> if JSON return json_parse_file()
> }
>
> We double the format switch, and also two functions per format are
> needed to do build and parsing.
> Total = 6 functions heavily duplicating code.
>
> With my approach, we have 1 function that decides which format to
> parse, and a one function per format to build and do parsing.
> Total = 3 functions, no duplicate code.
I guess this is saving you code, but I think this abstraction needs to
be refined, your functions:
* nft_mxml_do_build_tree
* nft_mxml_do_build_tree_file
look almost the same, only difference is mxmlLoadString / mxmlLoadFile.
I really think you can save more code and make this look better if you
rework the internal nft_*_json_parse functions to receive the xml/json
trees, eg.
static int nft_set_json_parse(struct nft_set *s, json_t tree
struct nft_parse_err *err)
{
...
}
So you nft_set_do_parse() creates the tree and pass it to it.
You can also add an enum like:
enum {
NFT_PARSE_BUFFER,
NFT_PARSE_FILE,
};
json_t *nft_json_build_tree(uint32_t type, void *data)
{
json_t *tree;
switch (type) {
case NFT_PARSE_BUFFER:
tree = nft_json_build_tree(data, ...);
break;
case NFT_PARSE_FILE:
tree = nft_json_build_tree_file(data, ...);
break;
}
return tree;
}
that you can pass this enum to nft_set_do_parse() to indicate how the
tree need to be build, eg.
statiuc int nft_set_do_parse(..., uint32_t format)
{
switch (format) {
...
case NFT_PARSE_JSON:
tree = nft_json_build_tree(type, data);
break;
...
}
...
}
Where data is the file descriptor / buffer area.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
