On 06/08/2011 05:18 PM, Michal Hocko wrote:
> On Wed 08-06-11 16:40:58, Jan Safranek wrote:
>> On 06/07/2011 02:34 PM, Michal Hocko wrote:
> [...]
>>>  /**
>>> + * Sets file permissions of the group's control and tasks files according
>>> + * to the configuration stored in the given @c cgroup structure.
>>> + * @param path Path to the group.
>>> + * @param cgroup
>>> + */
>>> +int cgroup_set_permissions(const char *path, struct cgroup *cgroup);
>>
>> I got probably misunderstood, what I wanted was
>>
>> cgroup_set_permissions(struct cgroup *cgroup,
>>      mode_t control_fperm,
>>      mode_t control_dperm,
>>      mode_t task_fperm);
>>
>> which would just set the values in the struct cgroup and
>> cgroup_create_cgroup() would just use them, similarly to
>> cgroup_set_uid_gid() in wrapper.c.
> 
> I have looked at that function but I found it more important to
> encapsulate the real logic into the function (open coding it would be OK
> if there was only one caller but we have 2 at the moment).

The goal I am trying to reach is the same logic (and API) for setting
owners of files/directories and for setting permissions of these files.
cgroup_set_permissions() and cgroup_set_uid_gid() should be functionally
similar, i.e. store values in struct cgroup and let cgroup_create_group
use them.

Your version of cgroup_set_permissions() requires applications to access
directly struct cgroup internals. But we don't expose it in our public
headers! There must be some function to set it.

One caller of your cgroup_set_permissions() is main() in cgcreate.c - it
can call my version of cgroup_set_permissions before
cgroup_create_cgroup(), just like it calls cgroup_set_uid_gid().

The second caller is cgroup_create_cgroup() itself, and it can call
cg_chmod_recursive_controller directly + later chmod the tasks file
(which your cgroup_set_permissions sets incorrectly, btw).

Jan

> 
> Well, wrapper for setting permissions can be created as well, though I
> do not consider it really that important. You basically have a single
> potential caller because parsing code sets only one thing at the time.
> 
> [...]
>>> +int cgroup_set_permissions(const char *path, struct cgroup *cgroup)
>>> +{
>>> +   int error;
>>> +
>>> +   error = cg_chmod_recursive_controller(path,
>>> +                   cgroup->control_dperm,
>>> +                   cgroup->control_dperm != NO_PERMS,
>>> +                   cgroup->control_fperm,
>>> +                   cgroup->control_fperm != NO_PERMS,
>>> +                   1);
>>
>> api.c:320:4: warning: passing argument 1 of
>> 'cg_chmod_recursive_controller' discards 'const' qualifier from pointer
>> target type [enabled by default]
> 
> OK, I will make it char * instead.
> 


------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel

Reply via email to