> Thanks for writing this patch. I see one minor problem and also have a
> question:
>
> Needs Fixing:
>
> 1. This patch introduces a side-effect to the function: the cwd is still
> changed when the function returns. It would be safer to remove this side-
> effect by wrapping the block in
>
> char * current_dir = g_get_current_dir();
>
> ...
>
> g_chdir (current_dir);
> g_free (current_dir);
Yes, I thought about that... But I didn't do that as it was the same way as g_spawn* does for example... However, it also sounds better to me. So here it is!
> 2. Putting four g_free() statements on a single line really itches. :-) Please
> use a newline between each.
Yes, right... I just kept the old style.
> 3. Should the chdir() call in the patch be a g_chdir()? I think the answer is
> 'no' but I'm unsure.
It's just the same, and AFAIK we won't use this but in linux, it should be the same :)
(it also would need to include something else).
> Thanks for writing this patch. I see one minor problem and also have a dir();
> question:
>
> Needs Fixing:
>
> 1. This patch introduces a side-effect to the function: the cwd is still
> changed when the function returns. It would be safer to remove this side-
> effect by wrapping the block in
>
> char * current_dir = g_get_current_
>
> ...
>
> g_chdir (current_dir);
> g_free (current_dir);
Yes, I thought about that... But I didn't do that as it was the same way as g_spawn* does for example... However, it also sounds better to me. So here it is!
> 2. Putting four g_free() statements on a single line really itches. :-) Please
> use a newline between each.
Yes, right... I just kept the old style.
> 3. Should the chdir() call in the patch be a g_chdir()? I think the answer is
> 'no' but I'm unsure.
It's just the same, and AFAIK we won't use this but in linux, it should be the same :)
(it also would need to include something else).