Re: [PATCH] libsmbclient APIs to set configuration file and Logging callback function

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

Re: [PATCH] libsmbclient APIs to set configuration file and Logging callback function

Samba - samba-technical mailing list
Added patch file,

Github PR link https://github.com/samba-team/samba/pull/99

On Mon, Aug 7, 2017 at 7:29 PM, Jim McDonough <[hidden email]> wrote:

> On 08/07/2017 06:53 AM, Puran Chand via samba-technical wrote:
> > Hi Jim,
> >
> > I have sent DCO to [hidden email] from corporate email and each
> > commit is also signed with corporate email.
> >
> >  I believe with these in place, the code can have Company copyright as
> per
> > Readme.contributing from github.com (
> > https://github.com/samba-team/samba/blob/master/README.contributing)
> >
> > Please let me know if I am missing something here and I will take it up
> > with right folks in the company.
> No, you're right, so I retract my statement.  Sorry for the confusion.
>
> Jim
> >
> > I appreciate all the help.
> >
> > Thanks.
> >
> > Regards
> >
> > Puran Chand
> >
> >
> >
> > On 07-Aug-2017 4:00 PM, "Jim McDonough" <[hidden email]> wrote:
> >
> > Hi,
> > On 08/06/2017 09:45 PM, Puran Chand via samba-technical wrote:
> >> This PR has libsmbclient APIs to set configruation file and logging
> >> callback function which allows the libsmbclient user to capture logs.
> >>
> >> https://github.com/samba-team/samba/pull/95
> >>
> >> Review is appreciated.
> > This is a non-technical review.  The above PR contains a corporate
> > copyright, which is explicitly disallowed by Samba policy.  I do realize
> > it can be difficult to get personal copyright in a large company, so I'm
> > sorry to ask you to find out how to do that.
> >
> > We are more than happy to give credit in the code.  This can be done by
> > maintaining the corporate email address, as it is in source3/utils/net.c:
> >
> >   Copyright (C) 2001 Steve French  ([hidden email])
> >   Copyright (C) 2001 Jim McDonough ([hidden email])
> >
> > We can also include a direct comment about the company contributing the
> > patch, just not a copyright.
> >
> > Thanks,
> > Jim
> >
>
>

--

*RegardsPuran Chand*

libsmbclient-set-smb.conf-set-log-callback.patch (18K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libsmbclient APIs to set configuration file and Logging callback function

Samba - samba-technical mailing list
Updated patch file.

Changes include
1. Removed copyright notices
2. Changed DEBUG(..) to DBG_* for logging

On Wed, Aug 16, 2017 at 5:41 PM, Puran Chand <[hidden email]> wrote:

> Added patch file,
>
> Github PR link https://github.com/samba-team/samba/pull/99
>
> On Mon, Aug 7, 2017 at 7:29 PM, Jim McDonough <[hidden email]> wrote:
>
>> On 08/07/2017 06:53 AM, Puran Chand via samba-technical wrote:
>> > Hi Jim,
>> >
>> > I have sent DCO to [hidden email] from corporate email and each
>> > commit is also signed with corporate email.
>> >
>> >  I believe with these in place, the code can have Company copyright as
>> per
>> > Readme.contributing from github.com (
>> > https://github.com/samba-team/samba/blob/master/README.contributing)
>> >
>> > Please let me know if I am missing something here and I will take it up
>> > with right folks in the company.
>> No, you're right, so I retract my statement.  Sorry for the confusion.
>>
>> Jim
>> >
>> > I appreciate all the help.
>> >
>> > Thanks.
>> >
>> > Regards
>> >
>> > Puran Chand
>> >
>> >
>> >
>> > On 07-Aug-2017 4:00 PM, "Jim McDonough" <[hidden email]> wrote:
>> >
>> > Hi,
>> > On 08/06/2017 09:45 PM, Puran Chand via samba-technical wrote:
>> >> This PR has libsmbclient APIs to set configruation file and logging
>> >> callback function which allows the libsmbclient user to capture logs.
>> >>
>> >> https://github.com/samba-team/samba/pull/95
>> >>
>> >> Review is appreciated.
>> > This is a non-technical review.  The above PR contains a corporate
>> > copyright, which is explicitly disallowed by Samba policy.  I do realize
>> > it can be difficult to get personal copyright in a large company, so I'm
>> > sorry to ask you to find out how to do that.
>> >
>> > We are more than happy to give credit in the code.  This can be done by
>> > maintaining the corporate email address, as it is in
>> source3/utils/net.c:
>> >
>> >   Copyright (C) 2001 Steve French  ([hidden email])
>> >   Copyright (C) 2001 Jim McDonough ([hidden email])
>> >
>> > We can also include a direct comment about the company contributing the
>> > patch, just not a copyright.
>> >
>> > Thanks,
>> > Jim
>> >
>>
>>
>
>
> --
>
> *RegardsPuran Chand*
>

0001-Added-libsmbclient-APIs-for-setting-configuration-fi.patch (17K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libsmbclient APIs to set configuration file and Logging callback function

Samba - samba-technical mailing list
Hi All,

Gentle reminder.

This patch has trivial changes (I think).
A quick review is highly appreciated.

Thanks

Puran

On Tue, Sep 5, 2017 at 1:06 AM, Puran Chand <[hidden email]> wrote:

> Updated patch file.
>
> Changes include
> 1. Removed copyright notices
> 2. Changed DEBUG(..) to DBG_* for logging
>
> On Wed, Aug 16, 2017 at 5:41 PM, Puran Chand <[hidden email]> wrote:
>
>> Added patch file,
>>
>> Github PR link https://github.com/samba-team/samba/pull/99
>>
>> On Mon, Aug 7, 2017 at 7:29 PM, Jim McDonough <[hidden email]> wrote:
>>
>>> On 08/07/2017 06:53 AM, Puran Chand via samba-technical wrote:
>>> > Hi Jim,
>>> >
>>> > I have sent DCO to [hidden email] from corporate email and
>>> each
>>> > commit is also signed with corporate email.
>>> >
>>> >  I believe with these in place, the code can have Company copyright as
>>> per
>>> > Readme.contributing from github.com (
>>> > https://github.com/samba-team/samba/blob/master/README.contributing)
>>> >
>>> > Please let me know if I am missing something here and I will take it up
>>> > with right folks in the company.
>>> No, you're right, so I retract my statement.  Sorry for the confusion.
>>>
>>> Jim
>>> >
>>> > I appreciate all the help.
>>> >
>>> > Thanks.
>>> >
>>> > Regards
>>> >
>>> > Puran Chand
>>> >
>>> >
>>> >
>>> > On 07-Aug-2017 4:00 PM, "Jim McDonough" <[hidden email]> wrote:
>>> >
>>> > Hi,
>>> > On 08/06/2017 09:45 PM, Puran Chand via samba-technical wrote:
>>> >> This PR has libsmbclient APIs to set configruation file and logging
>>> >> callback function which allows the libsmbclient user to capture logs.
>>> >>
>>> >> https://github.com/samba-team/samba/pull/95
>>> >>
>>> >> Review is appreciated.
>>> > This is a non-technical review.  The above PR contains a corporate
>>> > copyright, which is explicitly disallowed by Samba policy.  I do
>>> realize
>>> > it can be difficult to get personal copyright in a large company, so
>>> I'm
>>> > sorry to ask you to find out how to do that.
>>> >
>>> > We are more than happy to give credit in the code.  This can be done by
>>> > maintaining the corporate email address, as it is in
>>> source3/utils/net.c:
>>> >
>>> >   Copyright (C) 2001 Steve French  ([hidden email])
>>> >   Copyright (C) 2001 Jim McDonough ([hidden email])
>>> >
>>> > We can also include a direct comment about the company contributing the
>>> > patch, just not a copyright.
>>> >
>>> > Thanks,
>>> > Jim
>>> >
>>>
>>>
>>
>>
>> --
>>
>> *RegardsPuran Chand*
>>
>
>

0001-Added-libsmbclient-APIs-for-setting-configuration-fi.patch (17K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libsmbclient APIs to set configuration file and Logging callback function

Samba - samba-technical mailing list
On Wed, 2017-09-20 at 14:49 -0400, Puran Chand via samba-technical
wrote:
> Hi All,
>
> Gentle reminder.
>
> This patch has trivial changes (I think).
> A quick review is highly appreciated.

Shouldn't smbc_setLogCallback() also pass in the private pointer?

It should also be clearly documented that the impact of these is
global, not local the the libsmbclient context.

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: [PATCH] libsmbclient APIs to set configuration file and Logging callback function

Samba - samba-technical mailing list
Oops, missed that, working on it.

Regarding documentation, is it okay if I document this in libsmbclient.h?

Thanks

-Puran

On Wed, Sep 20, 2017 at 3:32 PM, Andrew Bartlett <[hidden email]> wrote:

> On Wed, 2017-09-20 at 14:49 -0400, Puran Chand via samba-technical
> wrote:
> > Hi All,
> >
> > Gentle reminder.
> >
> > This patch has trivial changes (I think).
> > A quick review is highly appreciated.
>
> Shouldn't smbc_setLogCallback() also pass in the private pointer?
>
> It should also be clearly documented that the impact of these is
> global, not local the the libsmbclient context.
>
> 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: [PATCH] libsmbclient APIs to set configuration file and Logging callback function

Samba - samba-technical mailing list
Hi,

Added private_ptr to smbc_setLogCallback().

Attached updated patch file.

- Puran

On Wed, Sep 20, 2017 at 4:00 PM, Puran Chand <[hidden email]> wrote:

> Oops, missed that, working on it.
>
> Regarding documentation, is it okay if I document this in libsmbclient.h?
>
> Thanks
>
> -Puran
>
> On Wed, Sep 20, 2017 at 3:32 PM, Andrew Bartlett <[hidden email]>
> wrote:
>
>> On Wed, 2017-09-20 at 14:49 -0400, Puran Chand via samba-technical
>> wrote:
>> > Hi All,
>> >
>> > Gentle reminder.
>> >
>> > This patch has trivial changes (I think).
>> > A quick review is highly appreciated.
>>
>> Shouldn't smbc_setLogCallback() also pass in the private pointer?
>>
>> It should also be clearly documented that the impact of these is
>> global, not local the the libsmbclient context.
>>
>> 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
>>
>>
>

0001-Added-libsmbclient-APIs-for-setting-configuration-fi.patch (18K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libsmbclient APIs to set configuration file and Logging callback function

Samba - samba-technical mailing list
On Wed, 2017-09-20 at 17:42 -0400, Puran Chand wrote:
> Hi, 
>
> Added private_ptr to smbc_setLogCallback().
>
> Attached updated patch file.

Can you add a test please?

Ideally examples/libsmbclient would be built and hooked in to the test
system, but that is probably too much work.  

There is a smaller test suite in:
source4/torture/libsmbclient/libsmbclient.c

Thanks,

Andrew Bartlett

> - Puran
>
> On Wed, Sep 20, 2017 at 4:00 PM, Puran Chand <[hidden email]>
> wrote:
> > Oops, missed that, working on it.
> >
> > Regarding documentation, is it okay if I document this in
> > libsmbclient.h?
> >
> > Thanks
> >
> > -Puran
> >
> > On Wed, Sep 20, 2017 at 3:32 PM, Andrew Bartlett <[hidden email]
> > g> wrote:
> > > On Wed, 2017-09-20 at 14:49 -0400, Puran Chand via samba-
> > > technical
> > > wrote:
> > > > Hi All,
> > > >
> > > > Gentle reminder.
> > > >
> > > > This patch has trivial changes (I think).
> > > > A quick review is highly appreciated.
> > >
> > > Shouldn't smbc_setLogCallback() also pass in the private pointer?
> > >
> > > It should also be clearly documented that the impact of these is
> > > global, not local the the libsmbclient context.
> > >
> > > Andrew Bartlett
> > > --
> > > Andrew Bartlett                       http://samba.org/~abartlet/
> > > Authentication Developer, Samba Team  http://samba.org
> > > Samba Developer, Catalyst IT          http://catalyst.net.nz/serv
> > > ices/samba
> > >
> > >
> >
> >
>
>
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT  
https://catalyst.net.nz/services/samba





Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libsmbclient APIs to set configuration file and Logging callback function

Samba - samba-technical mailing list
Hi Andrew,


Added following 2 statements in libsmbclient.c and a dummy debug_callback
function.

smbc_setConfiguration(ctx, "../examples/smb.conf.default");
smbc_setLogCallback(ctx, NULL, debug_callback);

./examples/smb.conf.default was the only configuration I can think of to
use for smbc_setConfiguration().

Please let me know if these changes are good to go with.

Attached path file as well.


On Wed, Sep 20, 2017 at 6:43 PM, Andrew Bartlett <[hidden email]> wrote:

> On Wed, 2017-09-20 at 17:42 -0400, Puran Chand wrote:
> > Hi,
> >
> > Added private_ptr to smbc_setLogCallback().
> >
> > Attached updated patch file.
>
> Can you add a test please?
>
> Ideally examples/libsmbclient would be built and hooked in to the test
> system, but that is probably too much work.
>
> There is a smaller test suite in:
> source4/torture/libsmbclient/libsmbclient.c
>
> Thanks,
>
> Andrew Bartlett
>
> > - Puran
> >
> > On Wed, Sep 20, 2017 at 4:00 PM, Puran Chand <[hidden email]>
> > wrote:
> > > Oops, missed that, working on it.
> > >
> > > Regarding documentation, is it okay if I document this in
> > > libsmbclient.h?
> > >
> > > Thanks
> > >
> > > -Puran
> > >
> > > On Wed, Sep 20, 2017 at 3:32 PM, Andrew Bartlett <[hidden email]
> > > g> wrote:
> > > > On Wed, 2017-09-20 at 14:49 -0400, Puran Chand via samba-
> > > > technical
> > > > wrote:
> > > > > Hi All,
> > > > >
> > > > > Gentle reminder.
> > > > >
> > > > > This patch has trivial changes (I think).
> > > > > A quick review is highly appreciated.
> > > >
> > > > Shouldn't smbc_setLogCallback() also pass in the private pointer?
> > > >
> > > > It should also be clearly documented that the impact of these is
> > > > global, not local the the libsmbclient context.
> > > >
> > > > Andrew Bartlett
> > > > --
> > > > Andrew Bartlett                       http://samba.org/~abartlet/
> > > > Authentication Developer, Samba Team  http://samba.org
> > > > Samba Developer, Catalyst IT          http://catalyst.net.nz/serv
> > > > ices/samba
> > > >
> > > >
> > >
> > >
> >
> >
> --
> Andrew Bartlett
> https://samba.org/~abartlet/
> Authentication Developer, Samba Team         https://samba.org
> Samba Development and Support, Catalyst IT
> https://catalyst.net.nz/services/samba
>
>
>
>
>

0001-Added-libsmbclient-APIs-for-setting-configuration-fi.patch (19K) Download Attachment