"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
> On Tue, Aug 12, 2008 at 07:18:00PM +0200, Jim Meyering wrote:
>> I've rewritten virFileLinkPointsTo to be a lot simpler,
>> and more importantly, it has far fewer failure points.
>> If anyone wants to preserve the original behavior that
>> makes it fail when the first parameter does not specify
>> a symlink, I can add that. The only difference in behavior
>> would be when the two files are hard-linked.
>
> Nah, that's not a difference we'd encounter with the way
> we use this API inside libvirt.
>
>> In any case, I'll remove the FIXME comment.
>
> Remove the virLog() function too - the caller should be
> responsible for reporting errors / logging if it so
> desires. Aside from that, ACK - this is a very nice
> simplification.
Glad to.
I would have done that initially, but was trying
to retain more of the initial semantics.
Thanks for the quick review.
Here's the new version:
/* Return nonzero if checkLink and checkDest
refer to the same file. Otherwise, return 0. */
int virFileLinkPointsTo(const char *checkLink,
const char *checkDest)
{
struct stat src_sb;
struct stat dest_sb;
return (stat (checkLink, &src_sb) == 0
&& stat (checkDest, &dest_sb) == 0
&& SAME_INODE (src_sb, dest_sb));
}
And the patch:
>From 3e7df57c236c7d04f4800b104a266949d4334fba Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Tue, 12 Aug 2008 19:07:46 +0200
Subject: [PATCH] rewrite virFileLinkPointsTo
* src/util.c (SAME_INODE): Define.
(virFileLinkPointsTo): Rewrite to be more portable and more efficient.
---
src/util.c | 104 ++++++------------------------------------------------------
1 files changed, 10 insertions(+), 94 deletions(-)
diff --git a/src/util.c b/src/util.c
index 9f056e0..fe701cf 100644
--- a/src/util.c
+++ b/src/util.c
@@ -397,107 +397,23 @@ int virFileHasSuffix(const char *str,
return STREQ(str + len - suffixlen, suffix);
}
-#ifndef __MINGW32__
+#define SAME_INODE(Stat_buf_1, Stat_buf_2) \
+ ((Stat_buf_1).st_ino == (Stat_buf_2).st_ino \
+ && (Stat_buf_1).st_dev == (Stat_buf_2).st_dev)
+/* Return nonzero if checkLink and checkDest
+ refer to the same file. Otherwise, return 0. */
int virFileLinkPointsTo(const char *checkLink,
const char *checkDest)
{
- char dest[PATH_MAX];
- char real[PATH_MAX];
- char checkReal[PATH_MAX];
- int n;
-
- /* read the link destination */
- if ((n = readlink(checkLink, dest, PATH_MAX)) < 0) {
- switch (errno) {
- case ENOENT:
- case ENOTDIR:
- return 0;
-
- case EINVAL:
- virLog("File '%s' is not a symlink\n",
- checkLink);
- return 0;
-
- }
- virLog("Failed to read symlink '%s': %s\n",
- checkLink, strerror(errno));
- return 0;
- } else if (n >= PATH_MAX) {
- virLog("Symlink '%s' contents too long to fit in buffer\n",
- checkLink);
- return 0;
- }
-
- dest[n] = '\0';
-
- /* make absolute */
- if (dest[0] != '/') {
- char dir[PATH_MAX];
- char tmp[PATH_MAX];
- char *p;
-
- strncpy(dir, checkLink, PATH_MAX);
- dir[PATH_MAX-1] = '\0';
-
- if (!(p = strrchr(dir, '/'))) {
- virLog("Symlink path '%s' is not absolute\n", checkLink);
- return 0;
- }
-
- if (p == dir) /* handle unlikely root dir case */
- p++;
-
- *p = '\0';
-
- if (virFileBuildPath(dir, dest, NULL, tmp, PATH_MAX) < 0) {
- virLog("Path '%s/%s' is too long\n", dir, dest);
- return 0;
- }
-
- strncpy(dest, tmp, PATH_MAX);
- dest[PATH_MAX-1] = '\0';
- }
-
- /* canonicalize both paths */
- if (!realpath(dest, real)) {
- virLog("Failed to expand path '%s' :%s\n", dest, strerror(errno));
- strncpy(real, dest, PATH_MAX);
- real[PATH_MAX-1] = '\0';
- }
-
- if (!realpath(checkDest, checkReal)) {
- virLog("Failed to expand path '%s' :%s\n", checkDest, strerror(errno));
- strncpy(checkReal, checkDest, PATH_MAX);
- checkReal[PATH_MAX-1] = '\0';
- }
-
- /* compare */
- if (STRNEQ(checkReal, real)) {
- virLog("Link '%s' does not point to '%s', ignoring\n",
- checkLink, checkReal);
- return 0;
- }
+ struct stat src_sb;
+ struct stat dest_sb;
- return 1;
+ return (stat (checkLink, &src_sb) == 0
+ && stat (checkDest, &dest_sb) == 0
+ && SAME_INODE (src_sb, dest_sb));
}
-#else /* !__MINGW32__ */
-
-/* Gnulib has an implementation of readlink which could be used
- * to implement this, but it requires LGPLv3.
- */
-
-int
-virFileLinkPointsTo (const char *checkLink ATTRIBUTE_UNUSED,
- const char *checkDest ATTRIBUTE_UNUSED)
-{
- virLog (_("%s: not implemented\n"), __FUNCTION__);
- return 0;
-}
-
-#endif /*! __MINGW32__ */
-
int virFileExists(const char *path)
{
struct stat st;
--
1.6.0.rc2.38.g413e06
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list