Bug in macro LOG_MAKEPRI in syslog.h

Bug in macro LOG_MAKEPRI in syslog.h

[Pepe Aracil]

[-- Attachment #1: Type: text/plain, Size: 2219 bytes --]

Hi all.


Now the macro is defined as:
#define LOG_MAKEPRI(f, p) (((f)<<3) | (p))
And must be:
#define LOG_MAKEPRI(f, p) ((f) | (p))

The next example posts wrong facility (LOG_LOCAL1) because LOG_LOCAL1 is
already rotated ( #define LOG_LOCAL1   (17<<3) ).

syslog (LOG_MAKEPRI(LOG_LOCAL1, LOG_ERROR),
        "Unable to make network connection to %s.  Error=%m", host);


Thanks.

-- 

*NOTA LEGAL*
Este correo electrónico y, en su caso, cualquier fichero anexo al mismo,
contiene información de carácter confidencial exclusivamente dirigida a su
destinatario y se encuentra protegido por Ley. Cualquier persona distinta
de su destinataria tiene prohibida su reproducción, uso, divulgación, copia
o impresión total o parcial. Si ha recibido este correo electrónico por
error, se ruega lo notifique de inmediato al remitente borrando el mensaje
original juntamente con sus ficheros anexos. Gracias.

De conformidad con lo establecido en la LOPD, NAYAR SYSTEMS SL garantiza la
adopción de las medidas necesarias para asegurar el tratamiento
confidencial de los datos de carácter personal. Así mismo le informamos de
la inclusión de sus datos en un fichero bajo la responsabilidad de NAYAR
SYSTEMS SL, con la finalidad de poder atender los compromisos derivados de
la relación que mantenemos con usted. Si lo desea, puede ejercer sus
derechos de acceso, rectificación, cancelación y oposición mediante un
escrito a la siguiente dirección: info@nayarsystems.com

*LEGAL NOTE*

This email and any attachments to it contains is confidential information
exclusively intended for the recipients. Any divulgation, copy or
distribution to third parties is prohibited without written permission of
NAYAR SYSTEMS SL. If you have received this e-mail in error, please notify
the sender immediately. In accordance with Law 15/1999 of 13 December on
the Protection of Personal Data, the NAYAR SYSTEMS SL guarantees that it
has adopted the necessary measures to ensure the confidential treatment of
personal information. We also inform you that you can exercise your access,
rectification, cancellation and opposition rights by send us a mail to:
info@nayarsystems.com

[-- Attachment #2: Type: text/html, Size: 3506 bytes --]

Re: Bug in macro LOG_MAKEPRI in syslog.h

[Rich Felker]

On Tue, Oct 06, 2015 at 09:06:49PM +0200, Pepe Aracil wrote:
> Hi all.
> 
> 
> Now the macro is defined as:
> #define LOG_MAKEPRI(f, p) (((f)<<3) | (p))
> And must be:
> #define LOG_MAKEPRI(f, p) ((f) | (p))
> 
> The next example posts wrong facility (LOG_LOCAL1) because LOG_LOCAL1 is
> already rotated ( #define LOG_LOCAL1   (17<<3) ).
> 
> syslog (LOG_MAKEPRI(LOG_LOCAL1, LOG_ERROR),
>         "Unable to make network connection to %s.  Error=%m", host);

LOG_MAKEPRI is not supposed to be used for this; it's not even a
standard API. The documented way to use syslog is to pass
LOG_LOCAL1|LOG_ERROR. See the specification at
http://pubs.opengroup.org/onlinepubs/9699919799/functions/syslog.html

I agree with your reasoning that the <<3 does not make sense, but
glibc and other historical systems, where the nonstandard macro was
copied from, do the same, and there does not seem to be any value in
offering an incompatible version of a nonstandard/legacy macro like
this. Presumably you're supposed to use LOG_FAC() first to do the >>3
if you want to use these legacy macros, as in:

LOG_MAKEPRI(LOG_FAC(LOG_LOCAL1), LOG_ERROR)

Yes this is ugly..

Rich

Re: Bug in macro LOG_MAKEPRI in syslog.h

[Rich Felker]

On Tue, Oct 06, 2015 at 05:09:31PM -0400, Rich Felker wrote:
> On Tue, Oct 06, 2015 at 09:06:49PM +0200, Pepe Aracil wrote:
> > Hi all.
> > 
> > 
> > Now the macro is defined as:
> > #define LOG_MAKEPRI(f, p) (((f)<<3) | (p))
> > And must be:
> > #define LOG_MAKEPRI(f, p) ((f) | (p))
> > 
> > The next example posts wrong facility (LOG_LOCAL1) because LOG_LOCAL1 is
> > already rotated ( #define LOG_LOCAL1   (17<<3) ).
> > 
> > syslog (LOG_MAKEPRI(LOG_LOCAL1, LOG_ERROR),
> >         "Unable to make network connection to %s.  Error=%m", host);
> 
> LOG_MAKEPRI is not supposed to be used for this; it's not even a
> standard API. The documented way to use syslog is to pass
> LOG_LOCAL1|LOG_ERROR. See the specification at
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/syslog.html
> 
> I agree with your reasoning that the <<3 does not make sense, but
> glibc and other historical systems, where the nonstandard macro was
> copied from, do the same, and there does not seem to be any value in
> offering an incompatible version of a nonstandard/legacy macro like
> this. Presumably you're supposed to use LOG_FAC() first to do the >>3
> if you want to use these legacy macros, as in:
> 
> LOG_MAKEPRI(LOG_FAC(LOG_LOCAL1), LOG_ERROR)
> 
> Yes this is ugly..

Hmm, Alexander Monakov just pointed me to this:

https://sourceware.org/bugzilla/show_bug.cgi?id=14347

So maybe we should change it. In any case I think it's a really bad
idea to use this macro since it was "wrong" for a very long time on
glibc (and on musl). The documented correct way to achieve the same
thing is simply the | operator.

Rich

Re: Bug in macro LOG_MAKEPRI in syslog.h

[Rich Felker]

On Tue, Oct 06, 2015 at 09:06:49PM +0200, Pepe Aracil wrote:
> Hi all.
> 
> 
> Now the macro is defined as:
> #define LOG_MAKEPRI(f, p) (((f)<<3) | (p))
> And must be:
> #define LOG_MAKEPRI(f, p) ((f) | (p))
> 
> The next example posts wrong facility (LOG_LOCAL1) because LOG_LOCAL1 is
> already rotated ( #define LOG_LOCAL1   (17<<3) ).
> 
> syslog (LOG_MAKEPRI(LOG_LOCAL1, LOG_ERROR),
>         "Unable to make network connection to %s.  Error=%m", host);

Upon further investigation and discussion, it looks like making this
change is the right thing to do. For example:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=729666

I kind of wonder if it would be better to do something like:

#define LOG_MAKEPRI(f, p) (((f)&-8) | (p))

in case programs expecting the old definition use the result of
LOG_FAC. On the other hand this is all cruft, and LOG_FAC is probably
wrong too (anyone have a BSD definition to compare against?) so it
might make sense to just start out with a minimal change.

Best would be to get programs to stop using these bogus and
problematic macros...

Rich