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

Dmitri Pal dpal at redhat.com
Tue Apr 3 13:50:22 UTC 2012


On 04/03/2012 08:01 AM, Stephen Gallagher wrote:
> 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.

The file is a part of the repo. It is checked into the source control. I
do not have control on its permissions.
I think it is the umask but not during execution but rather during git
clone.

Yes this area needs more thought but IMO in a separate patch. And may be
done differently. For example create a file on the fly and make sure it
has specific permissions. But this is way out of the scope of the
current patch. This is why I opened a separate ticket to handle it.
>
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel


-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120403/3d06c089/attachment.html>


More information about the sssd-devel mailing list