Code review comment for lp://staging/~3v1n0/libindicator/shortcut-path-key

Revision history for this message
Charles Kerr (charlesk) wrote :

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);

Comment only:

2. Putting four g_free() statements on a single line really itches. :-) Please use a newline between each.

3. Should the chdir() call in the patch be a g_chdir()? I think the answer is 'no' but I'm unsure.

review: Needs Fixing

« Back to merge proposal