[PATCH 06/13] s3/profiles: replace dup_sec_desc() usage

[Volker Lendecke]

On Tue, May 27, 2014 at 11:17:29AM +0200, David Disseldorp wrote:
> On Tue, 27 May 2014 10:47:20 +0200, Volker Lendecke wrote:
> 
> > On Tue, May 27, 2014 at 10:52:46AM +0200, David Disseldorp wrote:
> > > Thanks for the feedback Volker!
> > > 
> > > On Mon, 26 May 2014 21:58:06 +0200, Volker Lendecke wrote:
> > > 
> > > ...
> > > > >  	/* swap out the SIDs in the security descriptor */
> > > > >  
> > > > > -	if ( !(new_sd = dup_sec_desc( outfile->mem_ctx, nk->sec_desc->sec_desc )) ) {
> > > > > -		fprintf( stderr, "Failed to copy security descriptor!\n" );
> > > > > +	if (nk->sec_desc->sec_desc == NULL) {
> > > > > +		new_sd = NULL;
> > > > > +	} else {
> > > > > +		new_sd = security_descriptor_copy(outfile->mem_ctx,
> > > > > +						  nk->sec_desc->sec_desc);
> > > > > +	}
> > > > > +	if (new_sd == NULL) {
> > > > > +		fprintf(stderr, "Failed to copy security descriptor!\n");
> > > > 
> > > > This looks wrong in the case when nk->sec_desc->sec_desc was
> > > > NULL initially.
> > > 
> > > dup_sec_desc() returns NULL if @src is NULL, but for
> > > security_descriptor_copy() it's up to the caller to check for a NULL
> > > @src.
> > > This patch shouldn't change the behaviour of this code-path, unless I'm
> > > missing something?
> > 
> > Well, then at least the error message is confusing. When
> > nk->sec_desc->sec_desc was NULL, we did not copy anything.
> 
> Yes, the error message was confusing, and still is :)
> I could add the attached patch to the top of the series if you'd prefer.

Yes, thanks!

Do you have a git tree somewhere?

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de