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

classic Classic list List threaded Threaded
31 messages Options
12
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
There is a new 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

github-pr-set_log_callback_libsmbclient-112.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
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

github-pr-set_log_callback_libsmbclient-112.patch (18K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
This introduces an API in libsmbclient which lets user to set logging
callback function which will be called (if set) whenever anything is logged
in samba code.
This would be helpful for developers who are linking libsmbclient library
and want to direct the logs from samba to different log file.

Review appreciated.

- Puran

On Wed, Nov 29, 2017 at 2:19 PM, Github bot account via samba-technical <
[hidden email]> 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
>
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
On Wed, 2017-11-29 at 15:00 +0530, Puran Chand wrote:
> This introduces an API in libsmbclient which lets user to set logging callback function which will be called (if set) whenever anything is logged in samba code.
> This would be helpful for developers who are linking libsmbclient library and want to direct the logs from samba to different log file.
>
> Review appreciated.

Thanks.  could you just test that it actually works, by printing a
message and seeing it in the callback?  Set a flag back via the context
pointer and check it after the DEBUG(0, ("foo")) call.  Then please
test resetting it with a NULL pointer, not just to test that but also
so we have logging in the rest of the test :-).

I'm really sorry for how long this has taken.

Andrew Bartlett
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
Sure Andrew, I will test this and attach the sample code as well by Monday..

- Puran

On Sat, Dec 2, 2017 at 10:28 AM, Andrew Bartlett <[hidden email]> wrote:

> On Wed, 2017-11-29 at 15:00 +0530, Puran Chand wrote:
> > This introduces an API in libsmbclient which lets user to set logging
> callback function which will be called (if set) whenever anything is logged
> in samba code.
> > This would be helpful for developers who are linking libsmbclient
> library and want to direct the logs from samba to different log file.
> >
> > Review appreciated.
>
> Thanks.  could you just test that it actually works, by printing a
> message and seeing it in the callback?  Set a flag back via the context
> pointer and check it after the DEBUG(0, ("foo")) call.  Then please
> test resetting it with a NULL pointer, not just to test that but also
> so we have logging in the rest of the test :-).
>
> I'm really sorry for how long this has taken.
>
> Andrew Bartlett
> --
> Andrew Bartlett                       http://samba.org/~abartlet/
> Authentication Developer, Samba Team  http://samba.org
> Samba Developer, Catalyst IT          http://catalyst.net.nz/
> services/samba
>
>
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
To add more,
I have been using this in my code with samba 4.6.6 release, works well.
Except un-setting part is not tested.

I will test that and will update the patch if required.

On Sat, Dec 2, 2017 at 5:30 PM, Puran Chand <[hidden email]> wrote:

> Sure Andrew, I will test this and attach the sample code as well by
> Monday..
>
> - Puran
>
> On Sat, Dec 2, 2017 at 10:28 AM, Andrew Bartlett <[hidden email]>
> wrote:
>
>> On Wed, 2017-11-29 at 15:00 +0530, Puran Chand wrote:
>> > This introduces an API in libsmbclient which lets user to set logging
>> callback function which will be called (if set) whenever anything is logged
>> in samba code.
>> > This would be helpful for developers who are linking libsmbclient
>> library and want to direct the logs from samba to different log file.
>> >
>> > Review appreciated.
>>
>> Thanks.  could you just test that it actually works, by printing a
>> message and seeing it in the callback?  Set a flag back via the context
>> pointer and check it after the DEBUG(0, ("foo")) call.  Then please
>> test resetting it with a NULL pointer, not just to test that but also
>> so we have logging in the rest of the test :-).
>>
>> I'm really sorry for how long this has taken.
>>
>> Andrew Bartlett
>> --
>> Andrew Bartlett                       http://samba.org/~abartlet/
>> Authentication Developer, Samba Team  http://samba.org
>> Samba Developer, Catalyst IT          http://catalyst.net.nz/service
>> s/samba
>>
>>
>
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
On Sat, 2017-12-02 at 17:34 +0530, Puran Chand wrote:
> To add more, 
> I have been using this in my code with samba 4.6.6 release, works well. 
> Except un-setting part is not tested.
>
> I will test that and will update the patch if required.

To be clear, by 'test' I mean 'update the included automated torture
test'.

Thanks,

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
Got it.

On Sat, Dec 2, 2017 at 11:51 PM, Andrew Bartlett <[hidden email]> wrote:

> On Sat, 2017-12-02 at 17:34 +0530, Puran Chand wrote:
> > To add more,
> > I have been using this in my code with samba 4.6.6 release, works well.
> > Except un-setting part is not tested.
> >
> > I will test that and will update the patch if required.
>
> To be clear, by 'test' I mean 'update the included automated torture
> test'.
>
> Thanks,
>
> Andrew Bartlett
>
> --
> Andrew Bartlett                       http://samba.org/~abartlet/
> Authentication Developer, Samba Team  http://samba.org
> Samba Developer, Catalyst IT          http://catalyst.net.nz/
> services/samba
>
>
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
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

github-pr-set_log_callback_libsmbclient-112.patch (19K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
Hi Andrew,

Added code in libsmbclient.c to verify the API.
Please review.

On Mon, Dec 4, 2017 at 12:54 PM, Github bot account via samba-technical <
[hidden email]> 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
>
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
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 <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
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 <[hidden email]> 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 <
> [hidden email]> 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
>
>
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
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. :)

On Mon, Dec 4, 2017 at 4:08 PM, Puran Chand <[hidden email]> 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 <[hidden email]>
> 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
>> <[hidden email]> 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/service
>> s/samba
>>
>>
>
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
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

github-pr-set_log_callback_libsmbclient-112.patch (18K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
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

github-pr-set_log_callback_libsmbclient-112.patch (19K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
Done.
Thanks.

On Mon, Dec 4, 2017 at 11:08 PM, Andrew Bartlett <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
> 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 <[hidden email]> 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
>
>
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
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 <[hidden email]>

Can I get a second team reviewer please?

Thanks,

Andrew Bartlett

> On Mon, Dec 4, 2017 at 11:08 PM, Andrew Bartlett <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
Thanks Andrew.

I will fix it in few minutes.

On Tue, Dec 5, 2017 at 3:53 PM, Andrew Bartlett <[hidden email]> 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 <[hidden email]>
>
> Can I get a second team reviewer please?
>
> Thanks,
>
> Andrew Bartlett
>
> > On Mon, Dec 4, 2017 at 11:08 PM, Andrew Bartlett <[hidden email]>
> 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 <[hidden email]>
> 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 <
> [hidden email]> 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 <[hidden email]> 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
>
>
Reply | Threaded
Open this post in threaded view
|

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

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
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

github-pr-set_log_callback_libsmbclient-112.patch (19K) Download Attachment
12