[PATCH] SDAP: Check return value before using output arguments

[Lukas Slebodnik]

ehlo,

simple patch is attached.

LS

[Pavel Reichl]

On 05/30/2015 04:52 PM, Lukas Slebodnik wrote:
> ehlo,
>
> simple patch is attached.
>
> LS
>
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

LGTM, ci passed:
sssd-ci.duckdns.org/logs/job/16/47/summary.html

You might consider changing:
         if (debug_level & SSSDBG_TRACE_ALL) {
in the very same fuction to IS_DEBUG_SET, but I don't insist.

[Lukas Slebodnik]

On (01/06/15 10:40), Pavel Reichl wrote:
> On 05/30/2015 04:52 PM, Lukas Slebodnik wrote:
> > ehlo,
> >
> > simple patch is attached.
> >
> > LS
>
> LGTM, ci passed:
> sssd-ci.duckdns.org/logs/job/16/47/summary.html
>
> You might consider changing:
>        if (debug_level & SSSDBG_TRACE_ALL) {
> in the very same fuction to IS_DEBUG_SET, but I don't insist.

Could you explain how it is related to the patch?
The title is "Check return value before using output arguments"

The patch should do what it is written in commit message.
Irrelevant changes should be in different patch.

LS

[Pavel Reichl]

On 06/01/2015 12:56 PM, Lukas Slebodnik wrote:
> On (01/06/15 10:40), Pavel Reichl wrote:
> > On 05/30/2015 04:52 PM, Lukas Slebodnik wrote:
> > > ehlo,
> > >
> > > simple patch is attached.
> > >
> > > LS
> >
> > LGTM, ci passed:
> > sssd-ci.duckdns.org/logs/job/16/47/summary.html
> >
> > You might consider changing:
> >         if (debug_level & SSSDBG_TRACE_ALL) {
> > in the very same fuction to IS_DEBUG_SET, but I don't insist.
>
> Could you explain how it is related to the patch?
> The title is "Check return value before using output arguments"

It's not related. Not just me but also other reviewers sometimes ask 
author of the patch if he could fix some nitpick in adjacent code area. 
You don't want, fine don't do it.
You patch is OK.
ACK
> The patch should do what it is written in commit message.
> Irrelevant changes should be in different patch.
>
> LS
> _______________________________________________
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Lukas Slebodnik]

On (01/06/15 13:02), Pavel Reichl wrote:
> On 06/01/2015 12:56 PM, Lukas Slebodnik wrote:
> > On (01/06/15 10:40), Pavel Reichl wrote:
> > > On 05/30/2015 04:52 PM, Lukas Slebodnik wrote:
> > > > ehlo,
> > > >
> > > > simple patch is attached.
> > > >
> > > > LS
> > >
> > > LGTM, ci passed:
> > > sssd-ci.duckdns.org/logs/job/16/47/summary.html
> > >
> > > You might consider changing:
> > >        if (debug_level & SSSDBG_TRACE_ALL) {
> > > in the very same fuction to IS_DEBUG_SET, but I don't insist.
> >
> > Could you explain how it is related to the patch?
> > The title is "Check return value before using output arguments"
>
> It's not related. Not just me but also other reviewers sometimes ask author
> of the patch if he could fix some nitpick in adjacent code area. You don't

It's fine to fix nitpick(s) related to a patch.

Unrelated chnages should be fix in different patch.
Feel free to send patch.

Thank you for review.

LS

[Jakub Hrozek]

On Mon, Jun 01, 2015 at 01:02:14PM +0200, Pavel Reichl wrote:
> On 06/01/2015 12:56 PM, Lukas Slebodnik wrote:
> > On (01/06/15 10:40), Pavel Reichl wrote:
> > > On 05/30/2015 04:52 PM, Lukas Slebodnik wrote:
> > > > ehlo,
> > > >
> > > > simple patch is attached.
> > > >
> > > > LS
> > >
> > > LGTM, ci passed:
> > > sssd-ci.duckdns.org/logs/job/16/47/summary.html
> > >
> > > You might consider changing:
> > >        if (debug_level & SSSDBG_TRACE_ALL) {
> > > in the very same fuction to IS_DEBUG_SET, but I don't insist.
> >
> > Could you explain how it is related to the patch?
> > The title is "Check return value before using output arguments"
>
> It's not related. Not just me but also other reviewers sometimes ask author
> of the patch if he could fix some nitpick in adjacent code area. You don't
> want, fine don't do it.
> You patch is OK.
> ACK

* master: 176244cb1e9df96ce36d36556de1fd766582b1dc