Re: svn commit: r1696607 - in /httpd/httpd/trunk/modules/http2: h2_alt_svc.c h2_config.c h2_request.c h2_response.c

2015-08-19 Thread Christophe JAILLET

Hi,

in the 2 functions below, you could also use apr_pstrcat instead of a 
hard coded (20) for extra space needed by '[', ']', ... + 
apr_pcalloc/strlen/strcpy/strcat
This is cleaner, IMHO and less verbose (and faster but we don't really 
care here).


CJ

Le 19/08/2015 16:55, ic...@apache.org a écrit :

Author: icing
Date: Wed Aug 19 14:55:26 2015
New Revision: 1696607



Modified: httpd/httpd/trunk/modules/http2/h2_config.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_config.c?rev=1696607&r1=1696606&r2=1696607&view=diff
==
--- httpd/httpd/trunk/modules/http2/h2_config.c (original)
+++ httpd/httpd/trunk/modules/http2/h2_config.c Wed Aug 19 14:55:26 2015
@@ -58,7 +58,7 @@ static void *h2_config_create(apr_pool_t
  h2_config *conf = (h2_config *)apr_pcalloc(pool, sizeof(h2_config));
  
  const char *s = x? x : "unknown";

-char *name = (char *)apr_pcalloc(pool, strlen(prefix) + strlen(s) + 20);
+char *name = apr_pcalloc(pool, strlen(prefix) + strlen(s) + 20);
  strcpy(name, prefix);
  strcat(name, "[");
  strcat(name, s);
@@ -97,8 +97,7 @@ void *h2_config_merge(apr_pool_t *pool,
  h2_config *add = (h2_config *)addv;
  h2_config *n = (h2_config *)apr_pcalloc(pool, sizeof(h2_config));
  
-char *name = (char *)apr_pcalloc(pool,

-20 + strlen(add->name) + strlen(base->name));
+char *name = apr_pcalloc(pool, 20 + strlen(add->name) + 
strlen(base->name));
  strcpy(name, "merged[");
  strcat(name, add->name);
  strcat(name, ", ");






Re: svn commit: r1696592 - in /httpd/httpd/trunk/modules/http2: h2_alt_svc.c h2_conn.c h2_from_h1.c h2_request.c h2_switch.c

2015-08-19 Thread Stefan Eissing

> Am 19.08.2015 um 16:37 schrieb William A Rowe Jr :
>  h2_alt_svc *h2_alt_svc_parse(const char *s, apr_pool_t *pool) {
> -const char *sep = strchr(s, '=');
> +const char *sep = strchr((char *)s, '=');
> 
> 
> We solve these issues with ap_strchr_c, ap_strrchr_c, ap_strstr_c etc.  This 
> avoids the casts, which are always worth avoiding for other analysis tools 
> and inadvertent overwriting.

I live to learn. Changed in r1696607.

bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782





Re: svn commit: r1696592 - in /httpd/httpd/trunk/modules/http2: h2_alt_svc.c h2_conn.c h2_from_h1.c h2_request.c h2_switch.c

2015-08-19 Thread William A Rowe Jr
On Wed, Aug 19, 2015 at 9:13 AM,  wrote:

> Author: icing
> Date: Wed Aug 19 14:13:49 2015
> New Revision: 1696592
>
> URL: http://svn.apache.org/r1696592
> Log:
> mod_h2 compiles warning free in maintainer-mode
>
> --- httpd/httpd/trunk/modules/http2/h2_alt_svc.c (original)
> +++ httpd/httpd/trunk/modules/http2/h2_alt_svc.c Wed Aug 19 14:13:49 2015
> @@ -42,13 +42,13 @@ void h2_alt_svc_register_hooks(void)
>   * - do not use quotation marks
>   */
>  h2_alt_svc *h2_alt_svc_parse(const char *s, apr_pool_t *pool) {
> -const char *sep = strchr(s, '=');
> +const char *sep = strchr((char *)s, '=');
>


We solve these issues with ap_strchr_c, ap_strrchr_c, ap_strstr_c etc.
This avoids the casts, which are always worth avoiding for other analysis
tools and inadvertent overwriting.


Re: C89-ify a recent change to h2_io_set.c

2015-08-19 Thread Stefan Eissing

> Am 19.08.2015 um 15:33 schrieb Jim Jagielski :
> 
> That's the intent for --enable-maintainer-mode

Excellent. Thx!

bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782





Re: C89-ify a recent change to h2_io_set.c

2015-08-19 Thread Jim Jagielski
That's the intent for --enable-maintainer-mode

> On Aug 19, 2015, at 8:22 AM, Stefan Eissing  
> wrote:
> 
> Thanks Jean-Frederic.
> 
> Is there a way to ./configure - other than CFLAGS - so that all developers 
> can compile with similar subsets of features and warnings? I would like to 
> write code so that it does not generate warnings for the rest of you...
> 
> //Stefan
> 
>> Am 19.08.2015 um 14:00 schrieb jean-frederic clere :
>> 
>> On 08/19/2015 11:57 AM, NormW wrote:
>>> G/Evening (Is here)
>>> A very small tweak is proposed for:
>>> httpd-trunk\modules\http2\h2_io_set.c
>>> to keep the C89-ers in their seats.
>> 
>> thanks committed.
>> 
>> Cheers
>> 
>> Jean-Frederic
> 
> bytes GmbH
> Hafenweg 16, 48155 Münster, Germany
> Phone: +49 251 2807760. Amtsgericht Münster: HRB5782
> 
> 
> 



Re: C89-ify a recent change to h2_io_set.c

2015-08-19 Thread Stefan Eissing
Thanks Jean-Frederic.

Is there a way to ./configure - other than CFLAGS - so that all developers can 
compile with similar subsets of features and warnings? I would like to write 
code so that it does not generate warnings for the rest of you...

//Stefan

> Am 19.08.2015 um 14:00 schrieb jean-frederic clere :
> 
> On 08/19/2015 11:57 AM, NormW wrote:
>> G/Evening (Is here)
>> A very small tweak is proposed for:
>>  httpd-trunk\modules\http2\h2_io_set.c
>> to keep the C89-ers in their seats.
> 
> thanks committed.
> 
> Cheers
> 
> Jean-Frederic

bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782





Re: C89-ify a recent change to h2_io_set.c

2015-08-19 Thread jean-frederic clere

On 08/19/2015 11:57 AM, NormW wrote:

G/Evening (Is here)
A very small tweak is proposed for:
  httpd-trunk\modules\http2\h2_io_set.c
to keep the C89-ers in their seats.


thanks committed.

Cheers

Jean-Frederic


C89-ify a recent change to h2_io_set.c

2015-08-19 Thread NormW

G/Evening (Is here)
A very small tweak is proposed for:
 httpd-trunk\modules\http2\h2_io_set.c
to keep the C89-ers in their seats.

The svn diff looks like:
Index: modules/http2/h2_io_set.c
===
--- modules/http2/h2_io_set.c   (revision 1696548)
+++ modules/http2/h2_io_set.c   (working copy)
@@ -67,12 +67,13 @@
 /* we keep the array sorted by id, so lookup can be done
  * by bsearch.
  */
+h2_io **ps;
 h2_io key;
 h2_io *pkey = &key;

 memset(&key, 0, sizeof(key));
 key.id = stream_id;
-h2_io **ps = bsearch(&pkey, sp->list->elts, sp->list->nelts,
+ps = bsearch(&pkey, sp->list->elts, sp->list->nelts,
  sp->list->elt_size, h2_stream_id_cmp);
 return ps? *ps : NULL;
 }

I won't bore anyone who can follow that module with a patch.
Cheers,
Norm