GNOME Bugzilla – Bug 700756
GFile.new_for_path arguments misses (type filename) annotation
Last modified: 2016-06-15 17:24:51 UTC
See https://developer.gnome.org/gio/2.36/GFile.html#g-file-new-for-path
And so seem g_file_get_path, g_file_get_name, etc.
A patch would be welcome for this
Created attachment 245210 [details] [review] Patch I found some of the most obvious ones, I still have doubts about g_build_path. Is it intended as a general-purpose function for all types of strings?
(In reply to comment #3) > I still have doubts about g_build_path. > Is it intended as a general-purpose function for all types of strings? It's used for a non-filename in gutils.c. (If g_build_filename() gets annotated and g_build_path() does not, then the g_build_filename() docs need to be updated to not claim they behave the same way on UNIX.) g_path_is_absolute() is probably also used for non-filenames in places, but that can safely be just left unannotated, since it will still give the same result if you have to convert your filesystem-charset path into a UTF-8 path before calling it.
Created attachment 291622 [details] [review] gfileutils: Fix a signed/unsigned integer comparison Also use size_t rather than int, allowing for larger files to be handled.
Created attachment 291623 [details] [review] gio: Add missing (type filename) annotations These differentiate between strings in the GLib filename encoding, and strings in UTF-8.
Created attachment 291624 [details] [review] gfileutils: Add missing (type filename) annotations These differentiate between strings in the GLib filename encoding, and strings in UTF-8.
Comment on attachment 245210 [details] [review] Patch Updated patches. Mostly the same; I added a few missing annotations to gfileutils.c, removed them from g_file_new_for_commandline_arg[_and_cwd]() since they explicitly take UTF-8 args, but added them to g_application_command_line_get_cwd(). I think g_build_path() and g_build_basename() should both have the annotations. Any case where the functions are (I guess) being abused for non-filenames can always cast (or equivalent in the binding language) to avoid encoding conversion.
I've just pushed two patches in bug 767245 to add filename annotations which also covers these cases, so the annotation patches should be obsolete. The g_file_read_link still applies. But, as far as I can see int will be converted to uint in the comparison and when uint at the end of the loop overflows it will just start reading smaller chunks again, so while a bug, it still produces a valid result.
Comment on attachment 291622 [details] [review] gfileutils: Fix a signed/unsigned integer comparison Pushing this one in response to the almost-review. It's fairly trivial. Attachment 291622 [details] pushed as c914114 - gfileutils: Fix a signed/unsigned integer comparison
(In reply to Christoph Reiter (lazka) from comment #9) > I've just pushed two patches in bug 767245 to add filename annotations which > also covers these cases, so the annotation patches should be obsolete. > > The g_file_read_link still applies. But, as far as I can see int will be > converted to uint in the comparison and when uint at the end of the loop > overflows it will just start reading smaller chunks again, so while a bug, > it still produces a valid result. The patch to gfileutils still applies. There are two remaining additions in the other patch, which I still think are valid: - g_file_get_child() - g_file_resolve_relative_path() Updated patches coming, rebased on master.
Created attachment 329862 [details] [review] gio: Add missing (type filename) annotations These differentiate between strings in the GLib filename encoding, and strings in UTF-8.
Created attachment 329863 [details] [review] gfileutils: Add missing (type filename) annotations These differentiate between strings in the GLib filename encoding, and strings in UTF-8.
Review of attachment 329862 [details] [review]: lgtm
Review of attachment 329863 [details] [review]: lgtm
Thanks. Attachment 329862 [details] pushed as ac11666 - gio: Add missing (type filename) annotations Attachment 329863 [details] pushed as ec40e9d - gfileutils: Add missing (type filename) annotations