> Hi,
>
> this patch adds a library to map a Windows SID to a Unix uid or gid. My
> current plan is to used it for AD trusts on the client and the server
> side, this is why the interface allows different kind of memory
> allocators.
>
> bye,
> Sumit
Jan's mail never made it into my mailbox, so I reply to my original
mail.
Just a couple small things:
ipa_idmap.c
line 98: the memset is redundant
Since the memory allocator can be supplied by the user I would like to
make sure that the memory is initialized to 0 under all circumstances.
So it might be redundant in some cases, but important in other cases.
line 251: This is nitpicking on my side, but I would move this check
after the
next two trivial - in case one of them fails, this one won't have to be
executed
done
line 321: I'm not completely sure about the design here. Is it
possible
to
have two domains with the same SID and different ranges? If yes, then
there
should be continue on this line instead of return
no, this is not possible the SID is always assumed to be unique.
These warnings showed up when I tried to make docs:
/root/sssd/src/providers/ipa/ipa_idmap.h:185: warning: argument 'str' of
command @param is not found in the argument list of is_domain_sid(const
char
*sid)
/root/sssd/src/providers/ipa/ipa_idmap.h:185: warning: The following
parameters of is_domain_sid(const char *sid) are not documented:
parameter 'sid'
fixed
Other than this, the patch is fine. I wasn't able to perform more
testing (like
make check) since I just managed to delete my entire git repo and that
means
it's time to go home ;-)
Thank you for the review, new version attached.
bye,
Sumit