[PATCH]-Bad debug message when adding services without explicit dns_discovery_domain

[Ariel Barria]

https://fedorahosted.org/sssd/ticket/920

[Jan Zelený]

> https://fedorahosted.org/sssd/ticket/920

Nack,
please follow our coding guidelines:

http://www.freeipa.org/page/Coding_Style

In particular, I'm referring to the line length.

Thanks
Jan

[Jakub Hrozek]

On Tue, Jun 12, 2012 at 11:04:28AM +0200, Jan Zelený wrote:
> > https://fedorahosted.org/sssd/ticket/920
>
> Nack,
> please follow our coding guidelines:
>
> http://www.freeipa.org/page/Coding_Style
>
> In particular, I'm referring to the line length.
>
> Thanks
> Jan

Please also change the debug message to appropriate symbolic constant.

[Simo Sorce]

On Tue, 2012-06-12 at 11:11 +0200, Jakub Hrozek wrote:
> On Tue, Jun 12, 2012 at 11:04:28AM +0200, Jan Zelený wrote:
> > > https://fedorahosted.org/sssd/ticket/920
> >
> > Nack,
> > please follow our coding guidelines:
> >
> > http://www.freeipa.org/page/Coding_Style
> >
> > In particular, I'm referring to the line length.
> >
> > Thanks
> > Jan
>
> Please also change the debug message to appropriate symbolic constant.

s/message/level/ I guess :)

Simo.

[Ariel Barria]

thanks for comments
> From: [e-mail]
> To: [e-mail]
> Date: Tue, 12 Jun 2012 08:30:07 -0400
> Subject: Re: [SSSD] [PATCH]-Bad debug message when adding services without explicit dns_discovery_domain
>
> On Tue, 2012-06-12 at 11:11 +0200, Jakub Hrozek wrote:
> > On Tue, Jun 12, 2012 at 11:04:28AM +0200, Jan Zelený wrote:
> > > > https://fedorahosted.org/sssd/ticket/920
> > >
> > > Nack,
> > > please follow our coding guidelines:
> > >
> > > http://www.freeipa.org/page/Coding_Style
> > >
> > > In particular, I'm referring to the line length.
> > >
> > > Thanks
> > > Jan
> >
> > Please also change the debug message to appropriate symbolic constant.
>
> s/message/level/ I guess :)
>
> Simo.
>
> -- 
> Simo Sorce * Red Hat, Inc * New York
>
> sssd-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/sssd-devel

[Stephen Gallagher]

On Tue, 2012-06-12 at 23:20 -0500, Ariel Barria wrote:
> thanks for comments
> > From: [e-mail]
> > To: [e-mail]
> > Date: Tue, 12 Jun 2012 08:30:07 -0400
> > Subject: Re: [SSSD] [PATCH]-Bad debug message when adding services
>
> without explicit dns_discovery_domain
> > On Tue, 2012-06-12 at 11:11 +0200, Jakub Hrozek wrote:
> > > On Tue, Jun 12, 2012 at 11:04:28AM +0200, Jan Zelený wrote:
> > > > > https://fedorahosted.org/sssd/ticket/920
> > > >
> > > > Nack,
> > > > please follow our coding guidelines:
> > > >
> > > > http://www.freeipa.org/page/Coding_Style
> > > >
> > > > In particular, I'm referring to the line length.
> > > >
> > > > Thanks
> > > > Jan
> > >
> > > Please also change the debug message to appropriate symbolic
>
> constant.
> > s/message/level/ I guess :)

Nack

First of all, you're still not following our style guidelines. Please do
not use tabs anywhere, keep your lines to 79 charcacters and do not have
trailing spaces in the lines.

Now, for the meat of the problem. The message that you're printing is
incorrect. We don't want to print the literal strings "example.com" and
"DOMAINNAME".

"DOMAINNAME" should be replaced by the input  string sssd_domain.

Unfortunately, at this stage of evaluation we do not have the actual DNS
domain to print, so we'll have to replace "example.com" with "the
machine's DNS domain name".

[Ariel Barria]

thanks for comments.

From: [e-mail]
To: [e-mail]
Date: Wed, 13 Jun 2012 07:27:54 -0400
Subject: Re: [SSSD] [PATCH]-Bad debug message when adding services without explicit dns_discovery_domain

On Tue, 2012-06-12 at 23:20 -0500, Ariel Barria wrote:
> thanks for comments
> > From: [e-mail]
> > To: [e-mail]
> > Date: Tue, 12 Jun 2012 08:30:07 -0400
> > Subject: Re: [SSSD] [PATCH]-Bad debug message when adding services
>
> without explicit dns_discovery_domain
> > On Tue, 2012-06-12 at 11:11 +0200, Jakub Hrozek wrote:
> > > On Tue, Jun 12, 2012 at 11:04:28AM +0200, Jan Zelený wrote:
> > > > > https://fedorahosted.org/sssd/ticket/920
> > > >
> > > > Nack,
> > > > please follow our coding guidelines:
> > > >
> > > > http://www.freeipa.org/page/Coding_Style
> > > >
> > > > In particular, I'm referring to the line length.
> > > >
> > > > Thanks
> > > > Jan
> > >
> > > Please also change the debug message to appropriate symbolic
>
> constant.
> > s/message/level/ I guess :)

Nack

First of all, you're still not following our style guidelines. Please do
not use tabs anywhere, keep your lines to 79 charcacters and do not have
trailing spaces in the lines.

Now, for the meat of the problem. The message that you're printing is
incorrect. We don't want to print the literal strings "example.com" and
"DOMAINNAME".

"DOMAINNAME" should be replaced by the input  string sssd_domain.

Unfortunately, at this stage of evaluation we do not have the actual DNS
domain to print, so we'll have to replace "example.com" with "the
machine's DNS domain name".

_______________________________________________
sssd-devel mailing list
[e-mail]
https://fedorahosted.org/mailman/listinfo/sssd-devel

[Stephen Gallagher]

On Wed, 2012-06-13 at 23:58 -0500, Ariel Barria wrote:
> thanks for comments.

Nack, still.

You can't use server->srv_data->dns_domain here because it hasn't been
populated yet if dns_discovery_domain wasn't used. So this is going to
be NULL and will just print "(null)" on glibc systems or crash on
non-glibc systems. Please use the literal string "the machine's DNS
domain name" as I said before.
> From: [e-mail]
> To: [e-mail]
> Date: Wed, 13 Jun 2012 07:27:54 -0400
> Subject: Re: [SSSD] [PATCH]-Bad debug message when adding services
> without explicit dns_discovery_domain
>
> On Tue, 2012-06-12 at 23:20 -0500, Ariel Barria wrote:
> > thanks for comments
> > > From: [e-mail]
> > > To: [e-mail]
> > > Date: Tue, 12 Jun 2012 08:30:07 -0400
> > > Subject: Re: [SSSD] [PATCH]-Bad debug message when adding services
> >
> > without explicit dns_discovery_domain
> > > On Tue, 2012-06-12 at 11:11 +0200, Jakub Hrozek wrote:
> > > > On Tue, Jun 12, 2012 at 11:04:28AM +0200, Jan Zelený wrote:
> > > > > > https://fedorahosted.org/sssd/ticket/920
> > > > >
> > > > > Nack,
> > > > > please follow our coding guidelines:
> > > > >
> > > > > http://www.freeipa.org/page/Coding_Style
> > > > >
> > > > > In particular, I'm referring to the line length.
> > > > >
> > > > > Thanks
> > > > > Jan
> > > >
> > > > Please also change the debug message to appropriate symbolic
> >
> > constant.
> > > s/message/level/ I guess :)
>
> Nack
>
> First of all, you're still not following our style guidelines. Please do
> not use tabs anywhere, keep your lines to 79 charcacters and do not have
> trailing spaces in the lines.
>
> Now, for the meat of the problem. The message that you're printing is
> incorrect. We don't want to print the literal strings "example.com" and
> "DOMAINNAME".
>
> "DOMAINNAME" should be replaced by the input  string sssd_domain.
>
> Unfortunately, at this stage of evaluation we do not have the actual DNS
> domain to print, so we'll have to replace "example.com" with "the
> machine's DNS domain name".
>
> _______________________________________________ sssd-devel mailing
> list [e-mail]
> https://fedorahosted.org/mailman/listinfo/sssd-devel
> _______________________________________________
> sssd-devel mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/sssd-devel

[Ariel Barria]

sorry, I hadn't understood.
thanks.
------------------------------
Nack, still.

You can't use server->srv_data->dns_domain here because it hasn't been
populated yet if dns_discovery_domain wasn't used. So this is going to
be NULL and will just print "(null)" on glibc systems or crash on
non-glibc systems. Please use the literal string "the machine's DNS
domain name" as I said before.

[Stephen Gallagher]

On Fri, 2012-06-15 at 12:28 -0500, Ariel Barria wrote:
> sorry, I hadn't understood.
> thanks.

Nack. Please actually build and test your patches.

This is not valid C. You cannot do variable substitution within an ?:
construct.

You need to construct two different DEBUG messages. One where
discovery_domain is available, the other where it is not. Do not try to
bludgeon them into the same message.