Re: svn commit: r1698133 - in /httpd/httpd/trunk: include/httpd.h modules/http2/h2_switch.c server/protocol.c server/util.c

2015-09-02 Thread Yann Ylavic
On Thu, Aug 27, 2015 at 2:13 PM,   wrote:
>
> Modified: httpd/httpd/trunk/server/util.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1698133&r1=1698132&r2=1698133&view=diff
> ==
> --- httpd/httpd/trunk/server/util.c (original)
> +++ httpd/httpd/trunk/server/util.c Thu Aug 27 12:13:59 2015
> @@ -3149,15 +3149,23 @@ AP_DECLARE(char *) ap_get_exec_line(apr_
>  return apr_pstrndup(p, buf, k);
>  }
>
> -AP_DECLARE(int) ap_array_index(const apr_array_header_t *array, const char 
> *s)
> +AP_DECLARE(int) ap_array_index(const apr_array_header_t *array,
> +   const char *s,
> +   apr_size_t start)
>  {
> -int i;
> -for (i = 0; i < array->nelts; i++) {
> +apr_size_t i;

Maybe here:
if (start < 0) {
return -1;
}
?

> +for (i = start; i < array->nelts; i++) {
>  const char *p = APR_ARRAY_IDX(array, i, const char *);
>  if (!strcmp(p, s)) {
> -return i;
> +return (int)i;
>  }
>  }
>  return -1;
>  }


Re: svn commit: r1698133 - in /httpd/httpd/trunk: include/httpd.h modules/http2/h2_switch.c server/protocol.c server/util.c

2015-09-02 Thread Yann Ylavic
On Thu, Aug 27, 2015 at 5:06 PM, William A Rowe Jr  wrote:
>
> On Aug 27, 2015 7:14 AM,  wrote:
>>
>> Author: icing
>> Date: Thu Aug 27 12:13:59 2015
>> New Revision: 1698133
>>
>> URL: http://svn.apache.org/r1698133
>> Log:
>> giving ap_array_index a start parameter, adding ap_array_contains
>>
>
>>   */
>> -AP_DECLARE(int) ap_array_index(const apr_array_header_t *array, const
>> char *s);
>> +AP_DECLARE(int) ap_array_index(const apr_array_header_t *array,
>> +   const char *s,
>> +   apr_size_t start);
>
> You want the type of rv of _index to correspond to the start input to rv,
> no?  E.g.
>
> int n = -1, count = 0;
> while ((n = ap_array_index(arr, findtag, n + 1)) >= 0)
> ++count;
>
> sizeof(int) does not have to equal sizeof(apr_size_t).  But for indexes I
> believe it's fine.  The alternative is apr_ssize_t for both start arg and
> rv.

Or use the same type as array->nelts: int?
The conversion from unsigned to signed and (possible) sizeof mixing
looks incorrect to me.

Also, maybe ap_array_string_index would be a better name since arrays
contain any pointer (likewise
s/ap_array_contains/ap_array_has_string/g).
We could then create new ones for other types when/if necessary (or
add them to APR by appending a single r :)


Re: svn commit: r1698133 - in /httpd/httpd/trunk: include/httpd.h modules/http2/h2_switch.c server/protocol.c server/util.c

2015-08-27 Thread William A Rowe Jr
On Aug 27, 2015 7:14 AM,  wrote:
>
> Author: icing
> Date: Thu Aug 27 12:13:59 2015
> New Revision: 1698133
>
> URL: http://svn.apache.org/r1698133
> Log:
> giving ap_array_index a start parameter, adding ap_array_contains
>

>   */
> -AP_DECLARE(int) ap_array_index(const apr_array_header_t *array, const
char *s);
> +AP_DECLARE(int) ap_array_index(const apr_array_header_t *array,
> +   const char *s,
> +   apr_size_t start);

You want the type of rv of _index to correspond to the start input to rv,
no?  E.g.

int n = -1, count = 0;
while ((n = ap_array_index(arr, findtag, n + 1)) >= 0)
++count;

sizeof(int) does not have to equal sizeof(apr_size_t).  But for indexes I
believe it's fine.  The alternative is apr_ssize_t for both start arg and
rv.