Re: Question about APLOGNO
On Saturday 19 July 2014 20:04:09, Christophe JAILLET wrote: Using the following regex: ap_log_.?error.*(_ERR|_EMERG|_CRIT)[^A]*$ many places with missing APLOGNO can be found. There are some false positives because the [^A]* at the end of the regex is here to check, in a more or less good way, the absence of APLOGNO. Most common mistakes are when APLOGNO is on the following line. There is also the coccinelle script docs/log-message-tags/find- messages.cocci which is smarter than what can be done with regexes. However, it is quite possible that it needs updating to newer coccinelle versions and/or current development in httpd. docs/log- message-tags/README has a paragraph about its usage.
Re: Question about APLOGNO
Le 19/07/2014 22:44, William A. Rowe Jr. a écrit : If it violates 80 col formatting style rule, absolutely do not shift the APLOGNO macro to the first line. Sure. Moreover, when submitting patches, I'll take care to only propose things that can be backported easily. Changes relying on other changes, not backported yet, will either be submitted as individual patches or will remain in my tree. Same for changes that could generate conflict when merging other changes, should they be backported one day. Best regards, CJ
Re: Question about APLOGNO
I'd strongly encourage backporting, if accepted on 2.x branch. The APn code exists to find guidance through web, email archives and forum searches. Keeping these consistent between 2.4 and 2.next is crucial. It also ensures further backports apply without a host of future conflicts. Christophe JAILLET christophe.jail...@wanadoo.fr wrote: Le 19/07/2014 22:44, William A. Rowe Jr. a écrit : If it violates 80 col formatting style rule, absolutely do not shift the APLOGNO macro to the first line. Sure. Moreover, when submitting patches, I'll take care to only propose things that can be backported easily. Changes relying on other changes, not backported yet, will either be submitted as individual patches or will remain in my tree. Same for changes that could generate conflict when merging other changes, should they be backported one day. Best regards, CJ
Re: Question about APLOGNO
Hi I have proposed for backport for 2.4. See STATUS. http://svn.apache.org/r1611978 http://svn.apache.org/r1612068 should merge without any trouble and should not generate any conflict with code only in trunk, should it be backported one day. What I have submitted and not proposed for backport yet are: http://svn.apache.org/r1611980 -- depends of r1608202 http://svn.apache.org/r1611979 -- depends of r1610814 My may concern is to keep 2.4 and trunk as close as possible, but should I also see what can be backported to 2.2 ? Best regards, CJ Le 20/07/2014 15:25, William A. Rowe Jr. a écrit : I'd strongly encourage backporting, if accepted on 2.x branch. The APn code exists to find guidance through web, email archives and forum searches. Keeping these consistent between 2.4 and 2.next is crucial. It also ensures further backports apply without a host of future conflicts. Christophe JAILLET christophe.jail...@wanadoo.fr wrote: Le 19/07/2014 22:44, William A. Rowe Jr. a écrit : If it violates 80 col formatting style rule, absolutely do not shift the APLOGNO macro to the first line. Sure. Moreover, when submitting patches, I'll take care to only propose things that can be backported easily. Changes relying on other changes, not backported yet, will either be submitted as individual patches or will remain in my tree. Same for changes that could generate conflict when merging other changes, should they be backported one day. Best regards, CJ
Re: Question about APLOGNO
Le 20/07/2014 15:45, Marion Christophe JAILLET a écrit : My may concern is to keep 2.4 and trunk as close as possible, but should I also see what can be backported to 2.2 ? s/may/main/
Question about APLOGNO
Hi, I was wondering if logged message, at least APLOG_ERR or APLOG_EMERG and APLOG_CRIT, should all have a corresponding APLOGNO()? Using the following regex: ap_log_.?error.*(_ERR|_EMERG|_CRIT)[^A]*$ many places with missing APLOGNO can be found. There are some false positives because the [^A]* at the end of the regex is here to check, in a more or less good way, the absence of APLOGNO. Most common mistakes are when APLOGNO is on the following line. If you think that adding the APLOGNO would be improvement, would you like me to: 1) propose patches that put back APLOGNO on the same line as ap_log_.?error, which seems to be the most common 2) add APLOGNO missing for APLOG_ERR, APLOG_EMERG and APLOG_CRIT? 3) only add these APLOGNO for new modules (mod_ssl_ct and associated files need many) Best regards, CJ
Re: Question about APLOGNO
On Sat, Jul 19, 2014 at 2:04 PM, Christophe JAILLET christophe.jail...@wanadoo.fr wrote: Hi, I was wondering if logged message, at least APLOG_ERR or APLOG_EMERG and APLOG_CRIT, should all have a corresponding APLOGNO()? Using the following regex: ap_log_.?error.*(_ERR|_EMERG|_CRIT)[^A]*$ many places with missing APLOGNO can be found. There are some false positives because the [^A]* at the end of the regex is here to check, in a more or less good way, the absence of APLOGNO. Most common mistakes are when APLOGNO is on the following line. If you think that adding the APLOGNO would be improvement, would you like me to: 1) propose patches that put back APLOGNO on the same line as ap_log_.?error, which seems to be the most common 2) add APLOGNO missing for APLOG_ERR, APLOG_EMERG and APLOG_CRIT? IMO anything more important than DEBUG needs a number, but I think even many DEBUG messages have numbers. 3) only add these APLOGNO for new modules (mod_ssl_ct and associated files need many) Ouch, I will fix my mess; thanks for pointing this out! Best regards, CJ -- Born in Roswell... married an alien... http://emptyhammock.com/ http://edjective.org/
Re: Question about APLOGNO
On Sat, Jul 19, 2014 at 2:13 PM, Jeff Trawick traw...@gmail.com wrote: IMO anything more important than DEBUG needs a number, but I think even many DEBUG messages have numbers. +1 -- Eric Covener cove...@gmail.com
Re: Question about APLOGNO
Le 19/07/2014 20:04, Christophe JAILLET a écrit : I was wondering if logged message, at least APLOG_ERR or APLOG_EMERG and APLOG_CRIT, should all have a corresponding APLOGNO()? While updating code for that, I came across APLOG_NOERRNO which is defined as: /* APLOG_NOERRNO is ignored and should not be used. It will be * removed in a future release of Apache. */ #define APLOG_NOERRNO (APLOG_LEVELMASK + 1) The only use of it is in mod_macro. (+ only one use in coe.c) Why should it be removed in the future? Isn't it useful to self document the code when, for any reason, no APLOGNO should be appended ? Best regards, CJ
Re: Question about APLOGNO
On Sat, Jul 19, 2014 at 4:04 PM, Christophe JAILLET christophe.jail...@wanadoo.fr wrote: Why should it be removed in the future? Isn't it useful to self document the code when, for any reason, no APLOGNO should be appended ? I think you misinterpreted, it is not related to APLOGNO. You can see in the 1.3 tree that httpd would add errno and strerror(errno) to any message that didn't set this bit (similar to the way a non-zero apr_status_t parameter works in 2.x) Since there's no telling when your log entry actually relates to something set errno, you see |APLOG_NOERRNO jammed onto messages in older code. -- Eric Covener cove...@gmail.com
Re: Question about APLOGNO
Le 19/07/2014 22:16, Eric Covener a écrit : On Sat, Jul 19, 2014 at 4:04 PM, Christophe JAILLET christophe.jail...@wanadoo.fr wrote: Why should it be removed in the future? Isn't it useful to self document the code when, for any reason, no APLOGNO should be appended ? I think you misinterpreted, it is not related to APLOGNO. You can see in the 1.3 tree that httpd would add errno and strerror(errno) to any message that didn't set this bit (similar to the way a non-zero apr_status_t parameter works in 2.x) Since there's no telling when your log entry actually relates to something set errno, you see |APLOG_NOERRNO jammed onto messages in older code. Ok, I understand. mod_macro puzzled me because I saw it on lines without any APLOGNO. I'll remove them, then. Thanks for explanation. CJ
Re: Question about APLOGNO
Spanning lines in regex tests is trivial. If it violates 80 col formatting style rule, absolutely do not shift the APLOGNO macro to the first line. Christophe JAILLET christophe.jail...@wanadoo.fr wrote: Hi, I was wondering if logged message, at least APLOG_ERR or APLOG_EMERG and APLOG_CRIT, should all have a corresponding APLOGNO()? Using the following regex: ap_log_.?error.*(_ERR|_EMERG|_CRIT)[^A]*$ many places with missing APLOGNO can be found. There are some false positives because the [^A]* at the end of the regex is here to check, in a more or less good way, the absence of APLOGNO. Most common mistakes are when APLOGNO is on the following line. If you think that adding the APLOGNO would be improvement, would you like me to: 1) propose patches that put back APLOGNO on the same line as ap_log_.?error, which seems to be the most common 2) add APLOGNO missing for APLOG_ERR, APLOG_EMERG and APLOG_CRIT? 3) only add these APLOGNO for new modules (mod_ssl_ct and associated files need many) Best regards, CJ
