[Asterisk-Dev] Misbehaviour in ast_variable_browse

[Brian West]

I don't think this is needed because if you know the var you want why 
not use ast_variable_retrieve to just get it.

/b

On Feb 21, 2005, at 12:29 PM, Luis Vazquez wrote:

> Hello all,
> despite I have made many minor changes to asterisk code and some bugs 
> reports and fixes this is my first mail to the dev list (I was only a 
> "voyeur" here until now) so first of all I want to congratulate every 
> one at the list for the great list it's and to thank a lot for the 
> many things I have learned reading the mails here.
>
> I want to comment of what I consider is a inconsistency in the 
> behavior of the method ast_variable_browse respect to the (suposedly 
> analog, even said in the inline doc) ast_category_browse method in 
> config.c.
>
> The point is that with ast_category_browse(config, prev) you can 
> really iterate over the config sections with a code snippet like this:
>
> /************ works now****************/
>  cat = ast_category_browse(cfg, NULL);
>  while(cat) {
>    .....
>    cat = ast_category_browse(cfg, cat);
>  }
> /************************************/
>
> So it would be logic to be able to do the same with 
> ast_variable_browse, like this:
>
> /***** not posible now **************/
> var = ast_variable_browse(cfg, cat, NULL);
> while( var ) {
>  .........
>  var = ast_variable_browse(cfg, cat, var);
> }
> /*************************************/
>
> But this is not posible now because the actual prototype of 
> ast_variable_browse() is:
>
> > struct ast_variable *ast_variable_browse(struct ast_config *config, 
> char *category)
>
> and allways returns the root ast_variable in the given category, even 
> when the doc text in the header file config.h suggests it should work 
> as ast_category_browse.
> ...................................
> //! Goes through variables
> /*!
> * Somewhat similar in intent as the ast_category_browse.....
> */
> ..........................................
>
> The best solution I think would be to replace the prototype for 
> something like this:
>
> > struct ast_variable *ast_variable_browse(struct ast_config *config, 
> char *category, struct ast_variable *prev[=NULL])
>
> but because gcc (at least with the compile options used in asterisk) 
> do not allow default param values it would require to change all 
> places where ast_variable_browse() is used with the prior prototipe 
> (and meaning) with an extra NULL parameter like this:
> - var = ast_variable_browse(config, cat);
> + var = ast_variable_browse(config, cat, NULL);
>
>
> So to avoid this odd changes what I have done is to add a new function:
>
> struct ast_variable *ast_variable_next(struct ast_config *config, char 
> *category,struct ast_variable *prev);
>
> that returns the next ast_variable in a config category if prev is not 
> null, and the root var (as the current ast_variable_browse) if 
> prev==NULL.
>
> This approach allows to write code like this:
> /***************************************/
>  var = ast_variable_browse(cfg, cat);
>  while( var ) {
>    ............
>  var = ast_variable_next(cfg, cat, var);
>  }
> /***************************************/
> without changing the places where ast_variable_browse() was used 
> previously.
>
> This is needed when you want to retrieve all the vars in a section and 
> don't know the variable names in advance.
> I need this because I'm using a custom asterisk application to load 
> the configuration parameters used in the many pbx facility 
> applications (mostly perl or c++ agi scripts) into the asterisk db; 
> and I want to be able to add a new parameter without needing to 
> recompilate the loader application (what I should do to use 
> ast_variable_retrieve() because I need to know the variable names in 
> advance)
>
> Attached is the code with the proposed new ast_variable_next() 
> function.
>
> Best regards and I would like to receive any feedback and opinions 
> about this suggestion.
> Luis
>
>
>
>
>
> --- config.c.ipc	2005-01-27 17:04:46.000000000 -0300
> +++ config.c	2005-02-21 16:18:06.990108528 -0300
> @@ -149,6 +149,24 @@
>  	return NULL;
>  }
>
> +struct ast_variable *ast_variable_next(struct ast_config *config, 
> char *category,struct ast_variable *prev)
> +{
> +	struct ast_variable *v = NULL;
> +	v = ast_variable_browse(config, category);
> +	if(v) {
> +	  if(prev) {
> +		while (v) {
> +		  if (v == prev)
> +			return v->next;
> +		  v=v->next;
> +		}
> +	  } else {
> +		return v;
> +	  }
> +	}
> +	return NULL;
> +}
> +
>  char *ast_variable_retrieve(struct ast_config *config, char 
> *category, char *value)
>  {
>  	struct ast_variable *v;
> --- include/asterisk/config.h.orig	2004-03-02 04:57:06.000000000 -0300
> +++ include/asterisk/config.h	2005-02-21 16:13:08.389809756 -0300
> @@ -69,6 +69,14 @@
>   */
>  struct ast_variable *ast_variable_browse(struct ast_config *config, 
> char *category);
>
> +//! Iterates through variables
> +/*!
> + * Somewhat similar in intent as the ast_category_browse.  The prev 
> variable MUST be an actual pointer to an actual variable (such as one 
> obtained by using ast_variable_browse() or ast_variable_next() 
> itself).
> + * List variables of config file
> + * Returns next variable on the list, or NULL on failure
> + */
> +struct ast_variable *ast_variable_next(struct ast_config *config, 
> char *category,struct ast_variable *prev);
> +
>  //! Gets a variable
>  /*!
>   * \param config which (opened) config to use
> _______________________________________________
> Asterisk-Dev mailing list
> Asterisk-Dev at lists.digium.com
> http://lists.digium.com/mailman/listinfo/asterisk-dev
> To UNSUBSCRIBE or update options visit:
>    http://lists.digium.com/mailman/listinfo/asterisk-dev