Code review comment for lp://staging/~ralsina/ubuntuone-client/fix_712674

Revision history for this message
dobey (dobey) wrote :

77 + gchar buf[PATH_MAX + 1];
78 + gchar prefix[PATH_MAX + 1];
79 + g_snprintf(prefix,PATH_MAX,"%s/ubuntuone/shares/",g_get_user_data_dir());

This is just bad C. :)

You should use g_build_filename (); instead to do this. Also, don't fear the space key.

gchar * full_path = g_build_filename (g_get_uer_data_dir (), "ubuntuone", "shares", "", NULL);

And then g_free (full_path); before the return;

Also, can you change the is_shares variable in the code to be is_sharetome or something a little more descriptive and more than a single character different from is_shared as well?

90 + gchar *res = realpath(path, buf);
91 + if (g_str_has_prefix (res, prefix)) {
92 + is_shares = TRUE;
93 + }

This is also wrong here. The gchar *res needs to be declared at the beginning of the function. Also please call it real_path or resolved_path instead. And you should call realpath () with NULL as the second argument to use the return value. You should then g_free () the variable assigned to before returning from the function.

It also looks like there are maybe some spaces vs tabs issues in the spacing of the code, in the diff.
Also please do multi-line comments in the following style:

/*
 * Start of text
 */

review: Needs Fixing

« Back to merge proposal