Merge lp://staging/~ralsina/ubuntuone-client/fix_712674 into lp://staging/ubuntuone-client

grrr. "Check also in a subdirectory, with folders and files, and that the Publish option is enabled for contents outside the shares."

Fails to build ATM:

cc1: warnings being treated as errors
utils.c: In function ‘ubuntuone_is_folder_shares’:
utils.c:144:15: error: unused variable ‘f’

Please rename the function for this to something more than one character different from the _is_shared method. Documenting API is good, but so is having sane method names. :)

Also, please use --fixes=lp:712674 on at least one commit, so that the bug is linked and gets marked as fix committed when the branch lands.

Also, set a more descriptive commit message than "Fix bug ####" please. We don't need to put bug #s in commit messages, as bzr/launchpad have useful metadata for them that we should use.

> Please rename the function for this to something more than one character
> different from the _is_shared method. Documenting API is good, but so is
> having sane method names. :)

Done.

> Also, please use --fixes=lp:712674 on at least one commit, so that the bug is
> linked and gets marked as fix committed when the branch lands.
>
> Also, set a more descriptive commit message than "Fix bug ####" please. We
> don't need to put bug #s in commit messages, as bzr/launchpad have useful
> metadata for them that we should use.

I am used to a system where the link is done by just mentioning the bugs in the commit messages, sorry.

Fixed!

> Fails to build ATM:
>
> cc1: warnings being treated as errors
> utils.c: In function ‘ubuntuone_is_folder_shares’:
> utils.c:144:15: error: unused variable ‘f’

Missed a line I should have deleted.
Fixed!

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
 */

> 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.

Ok, didn't know g_build_filename.

>
> 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?

Sure.

> 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.

Yes. C++ism :-) Fixed.

> 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.

No. It always returns a pointer to the resolved path, the only difference is that since I used buf the pointer points to somewhere inside buf instead of allocating it using malloc. At least that's what the manual says.

I'll change it because you say it's ugly, but it's not *wrong* ;-)

> 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:

Ok, fixed.

> /*
> * Start of text
> */

Ok, fixed.

Looks great, no publishing in shares.