[PR PATCH] [Updated] Added smbc_SetLogCallback which lets third party code to capture libsmbclient logs

[Puran Chand]

Thanks Andrew.

I will fix it in few minutes.

On Tue, Dec 5, 2017 at 3:53 PM, Andrew Bartlett <abartlet at samba.org> wrote:

> On Tue, 2017-12-05 at 09:55 +0530, Puran Chand wrote:
> > Done.
> > Thanks.
>
> Thanks, and thanks for your patience!
>
> That looks pretty good to me.  Ideal would have also been to show that
> after the reset to (NULL, NULL) that the callback isn't called (the
> inverse test), and there is a small code style issue with:
>
> if(strstr(msg, TEST_STRING) != NULL){
>
> that should be
>
> if (strstr(msg, TEST_STRING) != NULL) {
>
> per README.Coding.
>
> These are just quibbles, and I can fix up the style if you don't get to
> it.  In the meantime:
>
> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
>
> Can I get a second team reviewer please?
>
> Thanks,
>
> Andrew Bartlett
>
> > On Mon, Dec 4, 2017 at 11:08 PM, Andrew Bartlett <abartlet at samba.org>
> wrote:
> > > On Mon, 2017-12-04 at 16:14 +0530, Puran Chand wrote:
> > > > There was major typo. Please re-read below 2 lines.
> > > >
> > > > Since the check cannot happen without torture_context that's why I
> am passing the same as private_ptr.
> > > > I don't see how these checks can be moved torture_libsmbclient_
> initialize().
> > > >
> > > > My apologies for the typo. :)
> > >
> > > So, what I would do is in debug_callback() just do something like:
> > >
> > > void debug_callback(void *private, const char *msg) {
> > >  bool *found = private;
> > >  if (strstr(msg, TEST_STRING) != NULL) {
> > >     *found = true;
> > >  }
> > > }
> > >
> > > Then you can do the torture logic outside the callback, and it can be
> > > called as many times (by other things you can't control) while just
> > > setting found once.  Then DEBUG(0, (TEST_STRING)) with and without the
> > > handler set, and check the private pointer to see if it was called.
> > > Naturally the second time it will print to the console, so please make
> > > TEST_STRING friendly.
> > >
> > > I hope this helps,
> > >
> > > Andrew Bartlett
> > >
> > > > On Mon, Dec 4, 2017 at 4:08 PM, Puran Chand <puran157 at gmail.com>
> wrote:
> > > > > Hi Andrew,
> > > > >
> > > > > I am bit confused here, please help me out.
> > > > >
> > > > > I am trying to compare the string received in debug_callback()
> when Debug(0, ("foo\n")) is called from torture_libsmbclient_initialize().
> > > > > Since the check cannot happen with torture_context that's why I am
> passing the same as private_ptr.
> > > > > I don't see how these checks cannot be moved torture_libsmbclient_
> initialize().
> > > > > I can remove those tests from debug_callback() and keep
> debug_callback() empty if that's what you mean here?
> > > > >
> > > > > Remainng review comments about adding "\n" to log and \\comments
> are addressed, I will put my changes after your response on above concern.
> > > > >
> > > > > - Puran
> > > > >
> > > > >
> > > > > On Mon, Dec 4, 2017 at 3:08 PM, Andrew Bartlett <
> abartlet at samba.org> wrote:
> > > > > > On Mon, 2017-12-04 at 14:29 +0530, Puran Chand wrote:
> > > > > > > Hi Andrew,
> > > > > > >
> > > > > > > Added code in libsmbclient.c to verify the API.
> > > > > > > Please review.
> > > > > >
> > > > > > Thanks.  I'm not quite sure what you are trying to do in
> > > > > > debug_callback(), you seem to be putting the test code in there,
> rather
> > > > > >  than in the test routine and just seeing if you got a flag back
> (via
> > > > > > the private pointer).  It should be just after the DEBUG() line.
> > > > > >
> > > > > > Also, on the buffering, you just need a \n and it will flush the
> > > > > > buffer, no need to fill it.
> > > > > >
> > > > > > You are on the right track.  For the final submission check
> > > > > > README.Coding and so avoid \\ comments.
> > > > > >
> > > > > > Thanks for giving this a go!
> > > > > >
> > > > > > Andrew Bartlett
> > > > > >
> > > > > > > On Mon, Dec 4, 2017 at 12:54 PM, Github bot account via
> samba-technical <samba-technical at lists.samba.org> wrote:
> > > > > > > > There is an updated pull request by puran157 against master
> on the Samba Samba Github repository
> > > > > > > >
> > > > > > > > https://github.com/puran157/samba
> set_log_callback_libsmbclient
> > > > > > > > https://github.com/samba-team/samba/pull/112
> > > > > > > >
> > > > > > > > Added smbc_SetLogCallback which lets third party code to
> capture libsmbclient logs
> > > > > > > >
> > > > > > > >
> > > > > > > > A patch file from https://github.com/samba-team/
> samba/pull/112.patch is attached
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > --
> > > > > > Andrew Bartlett
> http://samba.org/~abartlet/
> > > > > > Authentication Developer, Samba Team  http://samba.org
> > > > > > Samba Developer, Catalyst IT          http://catalyst.net.nz/
> services/samba
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > --
> > > Andrew Bartlett                       http://samba.org/~abartlet/
> > > Authentication Developer, Samba Team  http://samba.org
> > > Samba Developer, Catalyst IT          http://catalyst.net.nz/
> services/samba
> > >
> > >
> >
> >
> --
> Andrew Bartlett                       http://samba.org/~abartlet/
> Authentication Developer, Samba Team  http://samba.org
> Samba Developer, Catalyst IT          http://catalyst.net.nz/
> services/samba
>
>