Hi Ghee,
I totally agree your point, it's hard to review a patch's patch. So I
add some comments in this case, hope it help
thank you, Ghee, for your review and suggestion!
Jim
Ghee Teo wrote:
> This is just a general comment, can the submitter in general provide a
> short description of the fix.
> Especially when the patch is a patch to an existing patch. Since the
> idea of the review is
> to improve the quality of patches, and the idea of more eyes on a
> piece of code will help.
> However if the code is just a snipplet, with out some explanation text
> that would be hard.
> I don't believe anyone who does not own the component can make
> reasonable comment
> with this patch for example.
>
> So some minimum explanation text please in the future review patches
> :), please
>
> -Ghee
>
>
> Jim Li wrote:
>
>> Hey,
>>
>> Please review the following change to libgksu1.2-04-rabc-support.diff.
>> Welcome any suggestion and comments.
>>
>> Thanks,
>>
>> Jim
>> ------------------------------------------------------------------------
>>
>> Index: ChangeLog
>> ===================================================================
>> --- ChangeLog (revision 9602)
>> +++ ChangeLog (working copy)
>> @@ -1,3 +1,9 @@
>> +2006-10-30 Jim Li <jim.li at sun.com>
>> +
>> + * patches/libgksu1.2-04-rbac-support.diff:
>> + add echo command when try to validate user's password.
>> + fix a bug that it hang when it's invoded as a root.
>> +
>> 2006-10-30 Irene Huang <irene.huang at sun.com>
>>
>> * gnome-session.spec: add patch 10-gnome-volcheck-default-session.diff
>> Index: patches/libgksu1.2-04-rbac-support.diff
>> ===================================================================
>> --- patches/libgksu1.2-04-rbac-support.diff (revision 9602)
>> +++ patches/libgksu1.2-04-rbac-support.diff (working copy)
>> @@ -1,5 +1,5 @@
>> ---- libgksu1.2-1.3.1.orig/libgksu/gksu-context.c 2005-06-18
>> 22:16:33.000000000 +0800
>> -+++ libgksu1.2-1.3.1/libgksu/gksu-context.c 2006-10-19
>> 16:41:10.678082000 +0800
>> +--- libgksu1.2-1.3.1.orig/libgksu/gksu-context.c Sat Jun 18 22:16:33
>> 2005
>> ++++ libgksu1.2-1.3.1/libgksu/gksu-context.c Mon Oct 30 19:30:25 2006
>> @@ -23,7 +23,13 @@
>> #include <unistd.h>
>> #include <string.h>
>> @@ -329,7 +329,7 @@
>> if (!context->command)
>> {
>> -@@ -1496,3 +1697,977 @@
>> +@@ -1496,3 +1697,983 @@
>> return FALSE;
>> }
>> @@ -417,6 +417,12 @@
>
add some source code above this diff:
argcount = 0;
cmd[argcount] = g_strdup("/usr/lib/embedded_su");
>> + cmd[argcount] = g_strdup (context->user);
>> + argcount++;
>> +
>> ++ cmd[argcount] = g_strdup ("-c");
>> ++ argcount++;
>> ++
>> ++ cmd[argcount] = g_strdup ("echo > /dev/null");
>> ++ argcount++;
>> ++
>> + cmd[argcount] = NULL;
>> +
>> + pid = fork();
>
>
execn(cmd[0], cmd);
the goal of this paragraph code is to check if need password when try to
assume another user ro role ( context->user ), the old code doesn't
provide any command just like:
/usr/lib/embedded_su user
When it find the embedded_su need a password, it simply kill the child
process and return TRUE.
if embedded_su doesn't need a password, it should execute a null command
and immediately return instead of fork a shell and hang there
