Re: [Minios-devel] [UNIKRAFT PATCH 6/6] lib/posix-user: Add more group file related functions

[Costin Lupu <costin.lupu@xxxxxxxxx>]

On 12/16/19 4:24 PM, Simon Kuenzer wrote:
> On 06.12.19 14:41, Costin Lupu wrote:
>> We currently provide only a single group. This can be extended easily,
>> if it will be needed.
> 
> Can you add a short description what the implementation suppose to do
> and how it is implemented on a high level? It looks to me that you are
> adding the iteration.
> 

Ack.

>>
>> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
>> ---
>>   lib/posix-user/user.c | 64 +++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 64 insertions(+)
>>
>> diff --git a/lib/posix-user/user.c b/lib/posix-user/user.c
>> index 0d6ca3fb..004eee1b 100644
>> --- a/lib/posix-user/user.c
>> +++ b/lib/posix-user/user.c
>> @@ -60,6 +60,12 @@ UK_SLIST_HEAD(uk_entry_list, struct passwd_entry);
>>     static struct uk_entry_list passwds;
>>   +static void init_groups(void);
>> +
>> +/*
>> + * TODO make passwd management consistent with group management
>> + */
>> +
>>   void __constructor init_ukunistd()
>>   {
> 
> It is not part of this patch - but I just see it here: Can you use an
> Unikraft constructor (ukctortab) or Unikraft Initcall (ukinittab)
> instead? This is to make sure that this subsystem is initialized before
> any constructor of the application (preinit_array and initarray) is called.
> 

Ack.

>>       static struct passwd_entry p1;
>> @@ -77,6 +83,8 @@ void __constructor init_ukunistd()
>>         UK_SLIST_INIT(&passwds);
>>       UK_SLIST_INSERT_HEAD(&passwds, &p1, entries);
>> +
>> +    init_groups();
>>   }
>>     uid_t getuid(void)
>> @@ -328,3 +336,59 @@ int getgrnam_r(const char *name, struct group *grp,
>>         return 0;
>>   }
>> +
>> +struct group *getgrgid(gid_t gid)
>> +{
>> +    struct group *res;
>> +
>> +    if (gid == g__.gr_gid)
>> +        res = &g__;
>> +    else {
>> +        res = NULL;
>> +        errno = ENOENT;
>> +    }
>> +
>> +    return res;
>> +}
>> +
>> +static struct group_entry {
>> +    struct group *group;
>> +    UK_SLIST_ENTRY(struct group_entry) entries;
>> +} *groups_iter;
> 
> I think it will probably make sense to declare groups_iter as
> thread-local static variable as soon as HAVE_SCHED is defined (same
> would need to be done with users_iter). It should also be fine to put a
> comment for this as TODO for the future.
> 

I will fix this in v2.

>> +
>> +UK_SLIST_HEAD(uk_group_entry_list, struct group_entry);
>> +
>> +static struct uk_group_entry_list groups;
>> +
>> +static void init_groups(void)
>> +{
>> +    static struct group_entry ge;
>> +
>> +    ge.group = &g__;
>> +    UK_SLIST_INIT(&groups);
>> +    UK_SLIST_INSERT_HEAD(&groups, &ge, entries);
>> +}
>> +
>> +void setgrent(void)
>> +{
>> +    groups_iter = UK_SLIST_FIRST(&groups);
>> +}
>> +
>> +void endgrent(void)
>> +{
>> +    setgrent();
>> +}
>> +
>> +struct group *getgrent(void)
>> +{
>> +    struct group *res;
>> +
>> +    if (groups_iter) {
>> +        res = groups_iter->group;
>> +        groups_iter = UK_SLIST_NEXT(groups_iter, entries);
>> +    } else
>> +        res = NULL;
>> +
>> +    return res;
>> +
>> +}
>>
> 
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel