On Fri, 2006-03-17 at 22:04 +0100, Christian Neumair wrote:
> The attached patch now also handled NULL and "" paths better by
> canonicalizing them to "/", and it adds more DEBUG statements. Thanks to
> Priit Laes for pointing this glitch out.
+if (symlink[0] == '/' || !strcmp (path, "/"))
+ return g_strdup (symlink);
Why the strcmp? resolve("/", "blah") should be "/blah", not "blah". I
removed this.
p = strrchr (path, '/');
g_assert (p != NULL);
An assert is a bit harsh in this place, i made it just return symlink in
this case.
+ if (id != expected_id) {
+ g_critical ("ID mismatch (%u != %u)", id, expected_id);
Don't user g_critical for this, as its perfectly possible to return an
error code. g_critical should mainly be used instead of e.g. crashing
with a null-pointer dereference, it should only happen because the app
did something wrong, not because of something the server sent to us. I
realize the sftp code is full of g_criticals, but we shouldn't propagate
this more.
} else
DEBUG (g_log (G_LOG_DOMAIN, G_LOG_LEVEL_DEBUG,
"%s: Readlink failed, got unexpected
filename count (%d, 1
expected)",
__FUNCTION__, count));
This doesn't work well if DEBUG isn't defined. You need { } around it.
I fixed up all the above comments and commited it. However there are
still some issues with the symlink semantics:
+ /* we can't use FSTAT, because it always follows the symlink, but
doesn't return the target file name.
+ * So we fall back to our home-brewn LSTAT/readlink code. */
+ return get_file_info_for_path (handle->connection,
+ handle->path,
+ file_info, options);
For file: do_get_file_info_from_handle uses fstat, which also always
follows symlinks, so I think that is the right behaviour. We should do
the same in sftp:.
+ if (info->type == GNOME_VFS_FILE_TYPE_SYMBOLIC_LINK) {
+ path = g_build_filename (handle->path,
filename, NULL);
+ get_file_info_for_path (handle->connection,
path,
+ info,
handle->dir_options);
+ g_free (path);
+ } else
This needs to check FOLLOW_SYMLINKS in the options before following the
symlink.
Can you look into this?
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Alexander Larsson Red Hat, Inc
[EMAIL PROTECTED] [EMAIL PROTECTED]
He's a globe-trotting flyboy shaman with a robot buddy named Sparky. She's a
chain-smoking French-Canadian journalist descended from a line of powerful
witches. They fight crime!
_______________________________________________
gnome-vfs-list mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/gnome-vfs-list
