<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#ffffff" text="#000000">
    On 04/03/2012 08:01 AM, Stephen Gallagher wrote:
    <blockquote
      cite="mid:1333454518.13808.15.camel@sgallagh520.sgallagh.bos.redhat.com"
      type="cite">
      <pre wrap="">On Tue, 2012-04-03 at 10:08 +0200, Jan Zelený wrote:
</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre wrap="">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 <a class="moz-txt-link-freetext" href="https://fedorahosted.org/sssd/ticket/1284">https://fedorahosted.org/sssd/ticket/1284</a> 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.
</pre>
        </blockquote>
        <pre wrap="">
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.
</pre>
      </blockquote>
      <pre wrap="">
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.
</pre>
    </blockquote>
    <br>
    The file is a part of the repo. It is checked into the source
    control. I do not have control on its permissions.<br>
    I think it is the umask but not during execution but rather during
    git clone.<br>
    <br>
    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.<br>
    <blockquote
      cite="mid:1333454518.13808.15.camel@sgallagh520.sgallagh.bos.redhat.com"
      type="cite">
      <pre wrap="">
<fieldset class="mimeAttachmentHeader"></fieldset>
_______________________________________________
sssd-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a>
<a class="moz-txt-link-freetext" href="https://fedorahosted.org/mailman/listinfo/sssd-devel">https://fedorahosted.org/mailman/listinfo/sssd-devel</a>
</pre>
    </blockquote>
    <br>
    <br>
    <pre class="moz-signature" cols="72">-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
<a class="moz-txt-link-abbreviated" href="http://www.redhat.com/carveoutcosts/">www.redhat.com/carveoutcosts/</a>


</pre>
  </body>
</html>