Re: svn commit: r1874545 - /httpd/httpd/trunk/modules/filters/mod_brotli.c

2020-02-26 Thread Giovanni Bechis
On Wed, Feb 26, 2020 at 10:39:02PM +0100, Marion & Christophe JAILLET wrote:
> 
> Le 26/02/2020 à 18:47, gbec...@apache.org a écrit :
> > Author: gbechis
> > Date: Wed Feb 26 17:47:53 2020
> > New Revision: 1874545
> >
> > URL: http://svn.apache.org/viewvc?rev=1874545=rev
> > Log:
> > Avoid printing NULL strings in logs
> >
> > Modified:
> >  httpd/httpd/trunk/modules/filters/mod_brotli.c
> >
> > Modified: httpd/httpd/trunk/modules/filters/mod_brotli.c
> > URL: 
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_brotli.c?rev=1874545=1874544=1874545=diff
> > ==
> > --- httpd/httpd/trunk/modules/filters/mod_brotli.c (original)
> > +++ httpd/httpd/trunk/modules/filters/mod_brotli.c Wed Feb 26 17:47:53 2020
> > @@ -419,7 +419,7 @@ static apr_status_t compress_filter(ap_f
> >   }
> >   q = ap_get_token(r->pool, , 1);
> >   ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
> > -  "token: '%s' - q: '%s'", token, q);
> > +  "token: '%s' - q: '%s'", token ?: "NULL", q);
> 
> Is this syntax standard? This looks like a GNU extension.
> 
> Shouldn't we use
>     token ? token : "NULL"
> instead?
> 
> A few google search make me think that it could be an issue with VS 
> (i.e. windows build)
> 
> Just my 2c.
> 
> CJ
> 
you are right, committed thanks.
 Giovanni


signature.asc
Description: PGP signature


Re: svn commit: r1874545 - /httpd/httpd/trunk/modules/filters/mod_brotli.c

2020-02-26 Thread Marion & Christophe JAILLET



Le 26/02/2020 à 18:47, gbec...@apache.org a écrit :

Author: gbechis
Date: Wed Feb 26 17:47:53 2020
New Revision: 1874545

URL: http://svn.apache.org/viewvc?rev=1874545=rev
Log:
Avoid printing NULL strings in logs

Modified:
 httpd/httpd/trunk/modules/filters/mod_brotli.c

Modified: httpd/httpd/trunk/modules/filters/mod_brotli.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_brotli.c?rev=1874545=1874544=1874545=diff
==
--- httpd/httpd/trunk/modules/filters/mod_brotli.c (original)
+++ httpd/httpd/trunk/modules/filters/mod_brotli.c Wed Feb 26 17:47:53 2020
@@ -419,7 +419,7 @@ static apr_status_t compress_filter(ap_f
  }
  q = ap_get_token(r->pool, , 1);
  ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
-  "token: '%s' - q: '%s'", token, q);
+  "token: '%s' - q: '%s'", token ?: "NULL", q);


Is this syntax standard? This looks like a GNU extension.

Shouldn't we use
   token ? token : "NULL"
instead?

A few google search make me think that it could be an issue with VS 
(i.e. windows build)


Just my 2c.

CJ



  }
  
  /* No acceptable token found or q=0 */





Re: error on fedora31 when configure --enable-maintainer-mode

2020-02-26 Thread jean-frederic clere

On 26/02/2020 18:50, Giovanni Bechis wrote:

On 2/26/20 5:07 PM, jean-frederic clere wrote:

Hi,

I have 2 errors when compile on fedora31 with configure 
--enable-maintainer-mod. Attached a possible patch, comments?


Thanks,
they are both fixed in trunk in 2 separate commits, see 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64178.
  Giovanni



Proposed for back port ;-)

--
Cheers

Jean-Frederic


Re: error on fedora31 when configure --enable-maintainer-mode

2020-02-26 Thread Giovanni Bechis
On 2/26/20 5:07 PM, jean-frederic clere wrote:
> Hi,
> 
> I have 2 errors when compile on fedora31 with configure 
> --enable-maintainer-mod. Attached a possible patch, comments?
> 
Thanks,
they are both fixed in trunk in 2 separate commits, see 
https://bz.apache.org/bugzilla/show_bug.cgi?id=64178.
 Giovanni


error on fedora31 when configure --enable-maintainer-mode

2020-02-26 Thread jean-frederic clere

Hi,

I have 2 errors when compile on fedora31 with configure 
--enable-maintainer-mod. Attached a possible patch, comments?


--
Cheers

Jean-Frederic
Index: modules/filters/mod_brotli.c
===
--- modules/filters/mod_brotli.c(revision 1874543)
+++ modules/filters/mod_brotli.c(working copy)
@@ -419,7 +419,7 @@
 }
 q = ap_get_token(r->pool, , 1);
 ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
-  "token: '%s' - q: '%s'", token, q);
+  "token: '%s' - q: '%s'", token ? token : "", q);
 }
 
 /* No acceptable token found or q=0 */
Index: modules/filters/mod_deflate.c
===
--- modules/filters/mod_deflate.c   (revision 1874543)
+++ modules/filters/mod_deflate.c   (working copy)
@@ -730,7 +730,7 @@
 }
 q = ap_get_token(r->pool, , 1);
 ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
-  "token: '%s' - q: '%s'", token, q);
+  "token: '%s' - q: '%s'", token ? token : "", q);
 }
 
 /* No acceptable token found or q=0 */


Re: [PATCH 63628] Support specifying the http status codes to be considered by ProxyErrorOverride

2020-02-26 Thread Martin Drößler
Hi, 
any remarks regarding the new patch?
Are the changes acceptable, or are there still some improvements required?

Regards
MGD

Martin Drößler schrieb am 03.02.2020 18:33 (GMT +01:00):

> I attached a new/improved patch to the bug-ticket:
> https://bz.apache.org/bugzilla/show_bug.cgi?id=63628
> 
> 
> Regards
> Martin Drößler
> 
> Am 05.12.19 um 14:29 schrieb Eric Covener:
>> On Thu, Dec 5, 2019 at 7:51 AM Martin Drößler 
>> wrote:
>>>
>>> We're still in need of this feature.
>>> Is there anyone who can review the patch?
>> 
>> I think the proxy_util.c additions need an ap_ prefix and need to be
>> declared like all of the other non-static functions with AP_DECLARE.
>> 
>> The description and the manual seem to hide the use of this for
>> non-error codes while the diff seems to go out of its way to allow
>> non-error codes.
>> I think it should either be constrained in the diff or have some
>> notes/warnings/elaboration in the doc.
>> 
>> I personally do not like the use of two directives and the intercept
>> and override terminology mixing. I prefer that ProxyErrorOverride is
>> extended to accept ON or a list of status codes.
>> Another personal nit -- the name of the two added functions is not so
>> clear to me.
>> 
>> 
>> +int is_proxy_error_intercept_code(proxy_dir_conf *conf, int code)
>> +{
>> +if (apr_is_empty_array(conf->error_intercept_codes))
>> +return 0;
>> +
>> +proxy_status_code *list = (proxy_status_code *)
>> conf->error_intercept_codes->elts;
>> +
>> +int i;
>> ^^ not c89
>> 
>> 
>> Also, is_proxy_error_intercept_code could be static (and not in
>> mod_proxy.h) or just part of the other method since it is only called
>> from the other method.
>> 
>> 
>> 
>>>
>>> Regards
>>> Martin Drößler
>>>
>>> Martin Drößler schrieb am 16.09.2019 17:40 (GMT +02:00):
>>>
 Quick reminder.

 Martin Drößler schrieb am 22.08.2019 10:25:

> From: https://httpd.apache.org/dev/patches.html
>> Post to the developers list pointing out your patch and why you feel it 
>> is
>> important. Feel free to do this about once a week and continue until you
>> get
>> a
>> response.
>
> In this regard: the weekly friendly reminder.
>
>
> Regards
> Martin Drößler
>
> Martin Drößler schrieb am 13.08.2019 10:18:
>
>> Hi,
>>
>> one and a half week ago I submitted a patch/bugreport for this feature.
>> See: https://bz.apache.org/bugzilla/show_bug.cgi?id=63628
>>
>> And, as suggested by the how-to
>> (http://httpd.apache.org/dev/patches.html), I
>> wanted to ask about some feedback.
>>
>> It would definitely help me and my company to decide, if we can continue
>> with
>> our migration-project.
>>
>>
>> thanks,
>> Martin Drößler
>>
>

>>>
>> 
>> 
>