On Wed, Mar 25, 2009 at 07:33, Senthil Kumaran S <senthil_at_collab.net> wrote:
>...
> +++ trunk/subversion/libsvn_client/add.c Tue Mar 24 23:33:35 2009 (r36763)
> @@ -82,6 +82,51 @@ trim_string(char **pstr)
> str[i] = '\0';
> }
>
> +/* Split PROPERTY and store each individual value in PROPS.
> + Allocates from POOL. */
> +static void
> +split_props(apr_array_header_t **props,
> + const char *property,
> + apr_pool_t *pool)
> +{
> + apr_array_header_t *temp_props;
> + char *new_prop;
> + char *prop;
> + int i = 0;
> + int j = 0;
> +
> + temp_props = apr_array_make(pool, 4, sizeof(const char *));
> + new_prop = apr_palloc(pool, strlen(property));
allocate strlen(property)+1 .. with the suggested changes below, this
buffer will be used for all the results, and you may need the +1 to
store a NUL terminator.
> +
> + for (i = 0; i <= strlen(property); i++)
for (i = 0; property[i] != '\0'; i++)
would be better: you don't have to compute strlen() every iteration.
> + {
> + if (property[i] != ';')
> + {
> + new_prop[j] = property[i];
> + j++;
> + }
> + else if (property[i] == ';')
> + {
> + if (property[i+1] == ';')
> + {
> + new_prop[j] = property[i];
new_prop[j] = ';';
we already know its value, so a constant is better.
> + j++;
> + i++;
> + }
> + else
> + {
> + new_prop[j] = '\0';
> + prop = apr_palloc(pool, j+1);
> + j = 0;
> + strcpy(prop, new_prop);
> + APR_ARRAY_PUSH(temp_props, const char *) = prop;
there is no need to make an extra copy. you've allocated modifiable
memory already. just do the following:
new_prop[j] = '\0';
APR_ARRAY_PUSH(temp_props, const char *) = new_prop;
new_prop += j + 1;
j = 0;
> + }
> + }
> + }
> + APR_ARRAY_PUSH(temp_props, const char *) = new_prop;
> + *props = temp_props;
> +}
> +
> /* For one auto-props config entry (NAME, VALUE), if the filename pattern
> NAME matches BATON->filename case insensitively then add the properties
> listed in VALUE into BATON->properties.
> @@ -93,9 +138,9 @@ auto_props_enumerator(const char *name,
> void *baton,
> apr_pool_t *pool)
> {
> + int i;
> auto_props_baton_t *autoprops = baton;
> - char *property;
> - char *last_token;
> + apr_array_header_t *props;
>
> /* nothing to do here without a value */
> if (strlen(value) == 0)
> @@ -105,41 +150,44 @@ auto_props_enumerator(const char *name,
> if (apr_fnmatch(name, autoprops->filename, APR_FNM_CASE_BLIND) == APR_FNM_NOMATCH)
> return TRUE;
>
> - /* parse the value (we dup it first to effectively lose the
> - 'const', and to avoid messing up the original value) */
> - property = apr_pstrdup(autoprops->pool, value);
> - property = apr_strtok(property, ";", &last_token);
> - while (property)
> + props = apr_array_make(pool, 4, sizeof(const char *));
> + split_props(&props, value, autoprops->pool);
No need to allocate an array here -- split_props() creates and returns one.
> +
> + for (i = 0; i < props->nelts; i++)
> {
> int len;
> + char *new_token;
prop_name, or similar, would be a better name than "new_token"
> const char *this_value;
> - char *equal_sign = strchr(property, '=');
> + const char *token = APR_ARRAY_IDX(props, i, const char *);
> + char *equal_sign = strchr(token, '=');
>
> if (equal_sign)
> {
> *equal_sign = '\0';
> equal_sign++;
> trim_string(&equal_sign);
> + this_value = apr_palloc(autoprops->pool, strlen(equal_sign)+1);
> this_value = equal_sign;
what is that allocation for? it gets thrown out by the following line anyways
> }
> else
> {
> this_value = "";
> }
> - trim_string(&property);
> - len = strlen(property);
> + new_token = apr_pstrdup(autoprops->pool, token);
no need to dup this, it was already allocated in autoprops->pool.
> + trim_string(&new_token);
> + len = strlen(new_token);
> +
> if (len > 0)
> {
> svn_string_t *propval = svn_string_create(this_value,
> autoprops->pool);
No need to copy the contents again. Instead:
svn_string_t *propval = apr_palloc(autoprops->pool, sizeof(*propval));
propval->data = this_value;
propval->len = strlen(this_value);
>
> - apr_hash_set(autoprops->properties, property, len, propval);
> - if (strcmp(property, SVN_PROP_MIME_TYPE) == 0)
> + apr_hash_set(autoprops->properties, new_token, len, propval);
> + if (strcmp(new_token, SVN_PROP_MIME_TYPE) == 0)
> autoprops->mimetype = this_value;
> - else if (strcmp(property, SVN_PROP_EXECUTABLE) == 0)
> + else if (strcmp(new_token, SVN_PROP_EXECUTABLE) == 0)
> autoprops->have_executable = TRUE;
> }
> - property = apr_strtok(NULL, ";", &last_token);
> }
> return TRUE;
> }
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1411351
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1415500
Received on 2009-03-25 14:35:07 CET