summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFedor Indutny <fedor.indutny@gmail.com>2013-11-16 20:46:54 +0400
committerFedor Indutny <fedor.indutny@gmail.com>2013-12-10 23:17:00 +0400
commitf16edd2632930e3fbfead4d6d52eeac87824f1a6 (patch)
tree286ce474a141f3afae619922dc03384675c42d76
parent4a2792cd2f86403a71edf65d82600b6aad5713bf (diff)
downloadnode-f16edd2632930e3fbfead4d6d52eeac87824f1a6.tar.gz
fs: report correct path when EEXIST
When `symlink`, `link` or `rename` report EEXIST, ENOTEMPTY or EPERM - the destination file name should be included in the error message, instead of source file name. fix #6510
-rw-r--r--src/node_file.cc78
-rw-r--r--test/simple/test-fs-error-messages.js29
2 files changed, 89 insertions, 18 deletions
diff --git a/src/node_file.cc b/src/node_file.cc
index 54923b148..469ccbaae 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -50,14 +50,22 @@ using namespace v8;
class FSReqWrap: public ReqWrap<uv_fs_t> {
public:
+ void* operator new(size_t size, char* storage) { return storage; }
+
FSReqWrap(const char* syscall)
- : syscall_(syscall) {
+ : syscall_(syscall),
+ dest_len_(0) {
}
- const char* syscall() { return syscall_; }
+ inline const char* syscall() const { return syscall_; }
+ inline const char* dest() const { return dest_; }
+ inline unsigned int dest_len() const { return dest_len_; }
+ inline void dest_len(unsigned int dest_len) { dest_len_ = dest_len; }
private:
const char* syscall_;
+ unsigned int dest_len_;
+ char dest_[1];
};
@@ -102,6 +110,14 @@ static void After(uv_fs_t *req) {
argv[0] = UVException(req->errorno,
NULL,
req_wrap->syscall());
+ } else if ((req->errorno == UV_EEXIST ||
+ req->errorno == UV_ENOTEMPTY ||
+ req->errorno == UV_EPERM) &&
+ req_wrap->dest_len() > 0) {
+ argv[0] = UVException(req->errorno,
+ NULL,
+ req_wrap->syscall(),
+ req_wrap->dest());
} else {
argv[0] = UVException(req->errorno,
NULL,
@@ -212,10 +228,22 @@ struct fs_req_wrap {
};
-#define ASYNC_CALL(func, callback, ...) \
- FSReqWrap* req_wrap = new FSReqWrap(#func); \
- int r = uv_fs_##func(uv_default_loop(), &req_wrap->req_, \
- __VA_ARGS__, After); \
+#define ASYNC_DEST_CALL(func, callback, dest_path, ...) \
+ FSReqWrap* req_wrap; \
+ char* dest_str = (dest_path); \
+ int dest_len = dest_str == NULL ? 0 : strlen(dest_str); \
+ char* storage = new char[sizeof(*req_wrap) + dest_len]; \
+ req_wrap = new (storage) FSReqWrap(#func); \
+ req_wrap->dest_len(dest_len); \
+ if (dest_str != NULL) { \
+ memcpy(const_cast<char*>(req_wrap->dest()), \
+ dest_str, \
+ dest_len + 1); \
+ } \
+ int r = uv_fs_##func(uv_default_loop(), \
+ &req_wrap->req_, \
+ __VA_ARGS__, \
+ After); \
req_wrap->object_->Set(oncomplete_sym, callback); \
req_wrap->Dispatched(); \
if (r < 0) { \
@@ -227,13 +255,29 @@ struct fs_req_wrap {
} \
return scope.Close(req_wrap->object_);
-#define SYNC_CALL(func, path, ...) \
+#define ASYNC_CALL(func, callback, ...) \
+ ASYNC_DEST_CALL(func, callback, NULL, __VA_ARGS__) \
+
+#define SYNC_DEST_CALL(func, path, dest, ...) \
fs_req_wrap req_wrap; \
- int result = uv_fs_##func(uv_default_loop(), &req_wrap.req, __VA_ARGS__, NULL); \
+ int result = uv_fs_##func(uv_default_loop(), \
+ &req_wrap.req, \
+ __VA_ARGS__, \
+ NULL); \
if (result < 0) { \
int code = uv_last_error(uv_default_loop()).code; \
- return ThrowException(UVException(code, #func, "", path)); \
- }
+ if (dest != NULL && \
+ (code == UV_EEXIST || \
+ code == UV_ENOTEMPTY || \
+ code == UV_EPERM)) { \
+ return ThrowException(UVException(code, #func, "", dest)); \
+ } else { \
+ return ThrowException(UVException(code, #func, "", path)); \
+ } \
+ } \
+
+#define SYNC_CALL(func, path, ...) \
+ SYNC_DEST_CALL(func, path, NULL, __VA_ARGS__) \
#define SYNC_REQ req_wrap.req
@@ -431,9 +475,9 @@ static Handle<Value> Symlink(const Arguments& args) {
}
if (args[3]->IsFunction()) {
- ASYNC_CALL(symlink, args[3], *dest, *path, flags)
+ ASYNC_DEST_CALL(symlink, args[3], *dest, *dest, *path, flags)
} else {
- SYNC_CALL(symlink, *path, *dest, *path, flags)
+ SYNC_DEST_CALL(symlink, *path, *dest, *dest, *path, flags)
return Undefined();
}
}
@@ -451,9 +495,9 @@ static Handle<Value> Link(const Arguments& args) {
String::Utf8Value new_path(args[1]);
if (args[2]->IsFunction()) {
- ASYNC_CALL(link, args[2], *orig_path, *new_path)
+ ASYNC_DEST_CALL(link, args[2], *new_path, *orig_path, *new_path)
} else {
- SYNC_CALL(link, *orig_path, *orig_path, *new_path)
+ SYNC_DEST_CALL(link, *orig_path, *new_path, *orig_path, *new_path)
return Undefined();
}
}
@@ -482,14 +526,14 @@ static Handle<Value> Rename(const Arguments& args) {
if (len < 2) return TYPE_ERROR("new path required");
if (!args[0]->IsString()) return TYPE_ERROR("old path must be a string");
if (!args[1]->IsString()) return TYPE_ERROR("new path must be a string");
-
+
String::Utf8Value old_path(args[0]);
String::Utf8Value new_path(args[1]);
if (args[2]->IsFunction()) {
- ASYNC_CALL(rename, args[2], *old_path, *new_path)
+ ASYNC_DEST_CALL(rename, args[2], *new_path, *old_path, *new_path)
} else {
- SYNC_CALL(rename, *old_path, *old_path, *new_path)
+ SYNC_DEST_CALL(rename, *old_path, *new_path, *old_path, *new_path)
return Undefined();
}
}
diff --git a/test/simple/test-fs-error-messages.js b/test/simple/test-fs-error-messages.js
index 772966bd6..773faa69d 100644
--- a/test/simple/test-fs-error-messages.js
+++ b/test/simple/test-fs-error-messages.js
@@ -28,7 +28,10 @@ var assert = require('assert');
var path = require('path'),
fs = require('fs'),
fn = path.join(common.fixturesDir, 'non-existent'),
- existingFile = path.join(common.fixturesDir, 'exit.js');
+ existingFile = path.join(common.fixturesDir, 'exit.js'),
+ existingFile2 = path.join(common.fixturesDir, 'create-file.js'),
+ existingDir = path.join(common.fixturesDir, 'empty'),
+ existingDir2 = path.join(common.fixturesDir, 'keys');
// ASYNC_CALL
@@ -49,6 +52,10 @@ fs.link(fn, 'foo', function(err) {
assert.ok(0 <= err.message.indexOf(fn));
});
+fs.link(existingFile, existingFile2, function(err) {
+ assert.ok(0 <= err.message.indexOf(existingFile2));
+});
+
fs.unlink(fn, function(err) {
assert.ok(0 <= err.message.indexOf(fn));
});
@@ -57,6 +64,10 @@ fs.rename(fn, 'foo', function(err) {
assert.ok(0 <= err.message.indexOf(fn));
});
+fs.rename(existingDir, existingDir2, function(err) {
+ assert.ok(0 <= err.message.indexOf(existingDir2));
+});
+
fs.rmdir(fn, function(err) {
assert.ok(0 <= err.message.indexOf(fn));
});
@@ -136,6 +147,14 @@ try {
try {
++expected;
+ fs.linkSync(existingFile, existingFile2);
+} catch (err) {
+ errors.push('link');
+ assert.ok(0 <= err.message.indexOf(existingFile2));
+}
+
+try {
+ ++expected;
fs.unlinkSync(fn);
} catch (err) {
errors.push('unlink');
@@ -176,6 +195,14 @@ try {
try {
++expected;
+ fs.renameSync(existingDir, existingDir2);
+} catch (err) {
+ errors.push('rename');
+ assert.ok(0 <= err.message.indexOf(existingDir2));
+}
+
+try {
+ ++expected;
fs.readdirSync(fn);
} catch (err) {
errors.push('readdir');