[SSSD] [INI] Patches for ding-libs: Merging config sections, handling metadata, remaining Coverity issues...

Stephen Gallagher sgallagh at redhat.com
Tue Apr 3 12:01:58 UTC 2012


On Tue, 2012-04-03 at 10:08 +0200, Jan Zelený wrote:
> > Ok the whole set once more:
> > 
> > 1) Fixed the trailing spaces on patch 15 - they showed as warning when I
> > applied the patch in a different repo
> > 2) The problem with permissions that Jan have seen was caused by the
> > fact that when the repo is created as root the permissions on the files
> > that are used in tests are different. For now I added a different check.
> > In future there might be a need for some improvement in this area. I
> > created a task https://fedorahosted.org/sssd/ticket/1284 to track this
> > effort.
> > 3) Added two missing cleanup calls to patch 27 as Jan rightly indicated
> > that they were missing
> > 
> > All the rest is the same.
> 
> I'm not completely comfortable with your solution of the access check. You are 
> just assuming in the comment that the file was created by root and has 
> therefore different permissions. But what if it wasn't created by root and 
> still has these different permissions? I think the test could then let an error 
> in the code pass through. Please check that the file indeed belongs to root 
> just to be sure. Also error2 is not necessary IMO, you can re-user error.
> 
> This is the last minor change I'm asking for. If you want, I'll do it for you. 
> Other than that all patches are fine.

Actually, I think it's more likely that what's happening here is that
root and a casual user have a different default umask on most systems.
So this probably IS a bug in the code. You should be setting the umask
before creating the file (and resetting the umask to its old value
afterwards) in order to guarantee that it is always written with the
proper permissions.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120403/682f1a04/attachment.sig>


More information about the sssd-devel mailing list