[PATCH 1/1] idmapd: logging of Local-Realms only lists the last realm

[PATCH 1/1] idmapd: logging of Local-Realms only lists the last realm

[Steve Dickson]

From: Juno Krahn <Juno.Krahn@gmail.com>

The list of local realms can be logged with a massage like the following:
   rpc.idmapd: libnfsidmap: Realms list: 'EXAMPLE2.COM'
Instead of printing a list of realms, only the last realm in the list is shown.

https://bugzilla.redhat.com/show_bug.cgi?id=804152

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 libnfsidmap.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/libnfsidmap.c b/libnfsidmap.c
index 57bb6c3..641d766 100644
--- a/libnfsidmap.c
+++ b/libnfsidmap.c
@@ -285,8 +285,9 @@ int nfs4_init_name_mapping(char *conffile)
 			}
 			buf = malloc(siz);
 			if (buf) {
+				*buf = 0;
 				TAILQ_FOREACH(r, &local_realms->fields, link) {
-					sprintf(buf, "'%s' ", r->field);
+					sprintf(buf+strlen(buf), "'%s' ", r->field);
 				}
 				IDMAP_LOG(1, ("libnfsidmap: Realms list: %s", buf));
 				free(buf);
-- 
1.7.7.5

Re: [PATCH 1/1] idmapd: logging of Local-Realms only lists the last realm

[J. Bruce Fields]

On Mon, Mar 19, 2012 at 08:36:56AM -0400, Steve Dickson wrote:
> From: Juno Krahn <Juno.Krahn@gmail.com>
> 
> The list of local realms can be logged with a massage like the following:
>    rpc.idmapd: libnfsidmap: Realms list: 'EXAMPLE2.COM'
> Instead of printing a list of realms, only the last realm in the list is shown.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=804152

Makes sense.  Could we also move this whole thing into a separate
function?:

	if (idmap_verbosity >= 1)
		log_local_realms(local_realms);

It's all just debugging code, no sense cluttering up the main
function....--b.

> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  libnfsidmap.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/libnfsidmap.c b/libnfsidmap.c
> index 57bb6c3..641d766 100644
> --- a/libnfsidmap.c
> +++ b/libnfsidmap.c
> @@ -285,8 +285,9 @@ int nfs4_init_name_mapping(char *conffile)
>  			}
>  			buf = malloc(siz);
>  			if (buf) {
> +				*buf = 0;
>  				TAILQ_FOREACH(r, &local_realms->fields, link) {
> -					sprintf(buf, "'%s' ", r->field);
> +					sprintf(buf+strlen(buf), "'%s' ", r->field);
>  				}
>  				IDMAP_LOG(1, ("libnfsidmap: Realms list: %s", buf));
>  				free(buf);
> -- 
> 1.7.7.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] idmapd: logging of Local-Realms only lists the last realm

[Chuck Lever]

On Mar 19, 2012, at 8:36 AM, Steve Dickson wrote:

> From: Juno Krahn <Juno.Krahn@gmail.com>
> 
> The list of local realms can be logged with a massage like the following:
>   rpc.idmapd: libnfsidmap: Realms list: 'EXAMPLE2.COM'
> Instead of printing a list of realms, only the last realm in the list is shown.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=804152
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>

The patch says "From: Juno Krahn" but the sign-off is from you.  Should you also have an SOB from Juno?

> ---
> libnfsidmap.c |    3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/libnfsidmap.c b/libnfsidmap.c
> index 57bb6c3..641d766 100644
> --- a/libnfsidmap.c
> +++ b/libnfsidmap.c
> @@ -285,8 +285,9 @@ int nfs4_init_name_mapping(char *conffile)
> 			}
> 			buf = malloc(siz);
> 			if (buf) {
> +				*buf = 0;
> 				TAILQ_FOREACH(r, &local_realms->fields, link) {
> -					sprintf(buf, "'%s' ", r->field);
> +					sprintf(buf+strlen(buf), "'%s' ", r->field);
> 				}
> 				IDMAP_LOG(1, ("libnfsidmap: Realms list: %s", buf));
> 				free(buf);
> -- 
> 1.7.7.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 1/1] idmapd: logging of Local-Realms only lists the last realm

[Steve Dickson]

On 03/19/2012 10:05 AM, Chuck Lever wrote:
> 
> On Mar 19, 2012, at 8:36 AM, Steve Dickson wrote:
> 
>> From: Juno Krahn <Juno.Krahn@gmail.com>
>>
>> The list of local realms can be logged with a massage like the following:
>>   rpc.idmapd: libnfsidmap: Realms list: 'EXAMPLE2.COM'
>> Instead of printing a list of realms, only the last realm in the list is shown.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=804152
>>
>> Signed-off-by: Steve Dickson <steved@redhat.com>
> 
> The patch says "From: Juno Krahn" but the sign-off is from you. 
The patch came from:
    http://sourceforge.net/tracker/?func=detail&atid=903784&aid=3507122&group_id=183075 

Now in the past I have asked people to post patches to this list with
the appropriate format, but with a no-brainier like this patch, I
thought that would have been a waste time on both ends.    

>  Should you also have an SOB from Juno?
Juno gets credit for authoring the patch and I'm taking 
responsibility for the patch with my SOB... Does there
have to be any more process than that?? 

steved. 
 
> 
>> ---
>> libnfsidmap.c |    3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/libnfsidmap.c b/libnfsidmap.c
>> index 57bb6c3..641d766 100644
>> --- a/libnfsidmap.c
>> +++ b/libnfsidmap.c
>> @@ -285,8 +285,9 @@ int nfs4_init_name_mapping(char *conffile)
>> 			}
>> 			buf = malloc(siz);
>> 			if (buf) {
>> +				*buf = 0;
>> 				TAILQ_FOREACH(r, &local_realms->fields, link) {
>> -					sprintf(buf, "'%s' ", r->field);
>> +					sprintf(buf+strlen(buf), "'%s' ", r->field);
>> 				}
>> 				IDMAP_LOG(1, ("libnfsidmap: Realms list: %s", buf));
>> 				free(buf);
>> -- 
>> 1.7.7.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/1] idmapd: logging of Local-Realms only lists the last realm

[Chuck Lever]

On Mar 19, 2012, at 1:36 PM, Steve Dickson wrote:

> 
> 
> On 03/19/2012 10:05 AM, Chuck Lever wrote:
>> 
>> On Mar 19, 2012, at 8:36 AM, Steve Dickson wrote:
>> 
>>> From: Juno Krahn <Juno.Krahn@gmail.com>
>>> 
>>> The list of local realms can be logged with a massage like the following:
>>>  rpc.idmapd: libnfsidmap: Realms list: 'EXAMPLE2.COM'
>>> Instead of printing a list of realms, only the last realm in the list is shown.
>>> 
>>> https://bugzilla.redhat.com/show_bug.cgi?id=804152
>>> 
>>> Signed-off-by: Steve Dickson <steved@redhat.com>
>> 
>> The patch says "From: Juno Krahn" but the sign-off is from you. 
> The patch came from:
>    http://sourceforge.net/tracker/?func=detail&atid=903784&aid=3507122&group_id=183075 
> 
> Now in the past I have asked people to post patches to this list with
> the appropriate format, but with a no-brainier like this patch, I
> thought that would have been a waste time on both ends.
> 
>> Should you also have an SOB from Juno?
> Juno gets credit for authoring the patch and I'm taking 
> responsibility for the patch with my SOB... Does there
> have to be any more process than that?? 

My understanding is that SOB is not about responsibility, but about the provenance of the work.  It doesn't matter how large or small the patch is.  The SOB is a public declaration of Juno's desire to pass the patch to you.  Otherwise it looks like you didn't ask permission, even if you did.

I've never seen another project maintainer drop an SOB like this, so I'm just asking (and copying the list for other opinions).  No objection, just want to make sure we are dotting our open-source "i"s and so on.

> steved. 
> 
>> 
>>> ---
>>> libnfsidmap.c |    3 ++-
>>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>> 
>>> diff --git a/libnfsidmap.c b/libnfsidmap.c
>>> index 57bb6c3..641d766 100644
>>> --- a/libnfsidmap.c
>>> +++ b/libnfsidmap.c
>>> @@ -285,8 +285,9 @@ int nfs4_init_name_mapping(char *conffile)
>>> 			}
>>> 			buf = malloc(siz);
>>> 			if (buf) {
>>> +				*buf = 0;
>>> 				TAILQ_FOREACH(r, &local_realms->fields, link) {
>>> -					sprintf(buf, "'%s' ", r->field);
>>> +					sprintf(buf+strlen(buf), "'%s' ", r->field);
>>> 				}
>>> 				IDMAP_LOG(1, ("libnfsidmap: Realms list: %s", buf));
>>> 				free(buf);
>>> -- 
>>> 1.7.7.5
>>> 
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 1/1] idmapd: logging of Local-Realms only lists the last realm

[Steve Dickson]

Hey Chuck,

On 03/19/2012 01:43 PM, Chuck Lever wrote:
> 
> On Mar 19, 2012, at 1:36 PM, Steve Dickson wrote:
> 
>>
>>
>> On 03/19/2012 10:05 AM, Chuck Lever wrote:
>>>
>>> On Mar 19, 2012, at 8:36 AM, Steve Dickson wrote:
>>>
>>>> From: Juno Krahn <Juno.Krahn@gmail.com>
>>>>
>>>> The list of local realms can be logged with a massage like the following:
>>>>  rpc.idmapd: libnfsidmap: Realms list: 'EXAMPLE2.COM'
>>>> Instead of printing a list of realms, only the last realm in the list is shown.
>>>>
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=804152
>>>>
>>>> Signed-off-by: Steve Dickson <steved@redhat.com>
>>>
>>> The patch says "From: Juno Krahn" but the sign-off is from you. 
>> The patch came from:
>>    http://sourceforge.net/tracker/?func=detail&atid=903784&aid=3507122&group_id=183075 
>>
>> Now in the past I have asked people to post patches to this list with
>> the appropriate format, but with a no-brainier like this patch, I
>> thought that would have been a waste time on both ends.
>>
>>> Should you also have an SOB from Juno?
>> Juno gets credit for authoring the patch and I'm taking 
>> responsibility for the patch with my SOB... Does there
>> have to be any more process than that?? 
> 
> My understanding is that SOB is not about responsibility, but about the provenance of the work.
Hmm... I was thinking the Author tag would show more of the history 
of ownership than the SOB, but I'm not that much of a process guy... 

> It doesn't matter how large or small the patch is.  
True, but the complexity probably should. If ones pulling something out
of a bz to post upstream and its very simple patch and credit is given
to whom its due (both the Author and SOB tags exist on the posted patch)
I guess I just don't that as being a problem.

> The SOB is a public declaration of Juno's desire to pass the patch to you.
> Otherwise it looks like you didn't ask permission, even if you did.
I didn't, ask for Juno's SOB. Thats the point. I give Juno the
credit for writing the patch and I took responsibly for it. 
Basically trying to save Juno's time.
 
> 
> I've never seen another project maintainer drop an SOB like this, so I'm just asking (and copying the list for other opinions).  No objection, just want to make sure we are dotting our open-source "i"s and so on.
The SOB was never dropped because it never exist because I didn't ask 
for it. To your point, dropping an SOB would be bad and please point 
it out if I ever do so.

Again, I'm not a process guy... I like to get from A to B with the
least amount of pain. So if not asking for SOB from the author 
of a patch but I, the maintainer, is willing to add their SOB (i.e. 
willing to take responsibly) breaks an open-source rule, then I 
broke the rule. I must say though, I was under the impress that a 
SOB only had to exist, but again, process is not my forte ;-) 

steved.
 

Re: [PATCH 1/1] idmapd: logging of Local-Realms only lists the last realm

[J. Bruce Fields]

On Mon, Mar 19, 2012 at 01:43:52PM -0400, Chuck Lever wrote:
> My understanding is that SOB is not about responsibility, but about
> the provenance of the work.  It doesn't matter how large or small the
> patch is.

Well, it can matter in some ways.  For purposes of copyright, e.g., this
patch isn't likely to raise any issues.

> I've never seen another project maintainer drop an SOB like this, so
> I'm just asking (and copying the list for other opinions).  No
> objection, just want to make sure we are dotting our open-source "i"s
> and so on.

I have, for small stuff.  Or e.g. when a patch was written by a student
working with me and I saw previous revisions of it, so I'm willing to
vounch for it being their work.

And agreed with Steved on process--make it easy for the new and drive-by
contributors when it doesn't make extra work for the rest of us, and
they'll figure out the rest of the process later if they need to.

--b.

Re: [PATCH 1/1] idmapd: logging of Local-Realms only lists the last realm

[Steve Dickson]

On 03/19/2012 08:36 AM, Steve Dickson wrote:
> From: Juno Krahn <Juno.Krahn@gmail.com>
> 
> The list of local realms can be logged with a massage like the following:
>    rpc.idmapd: libnfsidmap: Realms list: 'EXAMPLE2.COM'
> Instead of printing a list of realms, only the last realm in the list is shown.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=804152
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
Committed...

steved
> ---
>  libnfsidmap.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/libnfsidmap.c b/libnfsidmap.c
> index 57bb6c3..641d766 100644
> --- a/libnfsidmap.c
> +++ b/libnfsidmap.c
> @@ -285,8 +285,9 @@ int nfs4_init_name_mapping(char *conffile)
>  			}
>  			buf = malloc(siz);
>  			if (buf) {
> +				*buf = 0;
>  				TAILQ_FOREACH(r, &local_realms->fields, link) {
> -					sprintf(buf, "'%s' ", r->field);
> +					sprintf(buf+strlen(buf), "'%s' ", r->field);
>  				}
>  				IDMAP_LOG(1, ("libnfsidmap: Realms list: %s", buf));
>  				free(buf);