Merge branch 'john/erts/fix-set-owner-group/ERL-589'

* john/erts/fix-set-owner-group/ERL-589:
  Fix file:change_group/change_owner

7 files changed, 47 insertions, 10 deletions

diff --git a/erts/emulator/nifs/common/prim_file_nif.c b/erts/emulator/nifs/common/prim_file_nif.c
index 6874f41d75..bbd9becb47 100644
--- a/erts/emulator/nifs/common/prim_file_nif.c
+++ b/erts/emulator/nifs/common/prim_file_nif.c
@@ -891,10 +891,10 @@ static ERL_NIF_TERM set_owner_nif(ErlNifEnv *env, int argc, const ERL_NIF_TERM a
     posix_errno_t posix_errno;
 
     efile_path_t path;
-    Uint32 uid, gid;
+    Sint32 uid, gid;
 
-    if(argc != 3 || !enif_get_uint(env, argv[1], &uid)
-                 || !enif_get_uint(env, argv[2], &gid)) {
+    if(argc != 3 || !enif_get_int(env, argv[1], &uid)
+                 || !enif_get_int(env, argv[2], &gid)) {
         return enif_make_badarg(env);
     }
 
diff --git a/erts/emulator/nifs/common/prim_file_nif.h b/erts/emulator/nifs/common/prim_file_nif.h
index cc9bc8f5c3..4194cdc7d9 100644
--- a/erts/emulator/nifs/common/prim_file_nif.h
+++ b/erts/emulator/nifs/common/prim_file_nif.h
@@ -177,7 +177,7 @@ posix_errno_t efile_set_permissions(const efile_path_t *path, Uint32 permissions
 
 /** @brief On Unix, this will set the owner/group to the given values. It will
  * do nothing on other platforms. */
-posix_errno_t efile_set_owner(const efile_path_t *path, Uint32 owner, Uint32 group);
+posix_errno_t efile_set_owner(const efile_path_t *path, Sint32 owner, Sint32 group);
 
 /** @brief Resolves the final path of the given link. */
 posix_errno_t efile_read_link(ErlNifEnv *env, const efile_path_t *path, ERL_NIF_TERM *result);
diff --git a/erts/emulator/nifs/unix/unix_prim_file.c b/erts/emulator/nifs/unix/unix_prim_file.c
index 4a6c476882..1637f9cb71 100644
--- a/erts/emulator/nifs/unix/unix_prim_file.c
+++ b/erts/emulator/nifs/unix/unix_prim_file.c
@@ -687,7 +687,7 @@ posix_errno_t efile_set_permissions(const efile_path_t *path, Uint32 permissions
     return 0;
 }
 
-posix_errno_t efile_set_owner(const efile_path_t *path, Uint32 owner, Uint32 group) {
+posix_errno_t efile_set_owner(const efile_path_t *path, Sint32 owner, Sint32 group) {
     if(chown((const char*)path->data, owner, group) < 0) {
         return errno;
     }
diff --git a/erts/emulator/nifs/win32/win_prim_file.c b/erts/emulator/nifs/win32/win_prim_file.c
index 9b79182f2c..8058350b25 100644
--- a/erts/emulator/nifs/win32/win_prim_file.c
+++ b/erts/emulator/nifs/win32/win_prim_file.c
@@ -801,7 +801,7 @@ posix_errno_t efile_set_permissions(const efile_path_t *path, Uint32 permissions
     return windows_to_posix_errno(GetLastError());
 }
 
-posix_errno_t efile_set_owner(const efile_path_t *path, Uint32 owner, Uint32 group) {
+posix_errno_t efile_set_owner(const efile_path_t *path, Sint32 owner, Sint32 group) {
     (void)path;
     (void)owner;
     (void)group;
diff --git a/erts/preloaded/ebin/prim_file.beam b/erts/preloaded/ebin/prim_file.beam
index 902b0945c6..9cc22222db 100644
--- a/erts/preloaded/ebin/prim_file.beam
+++ b/erts/preloaded/ebin/prim_file.beam
diff --git a/erts/preloaded/src/prim_file.erl b/erts/preloaded/src/prim_file.erl
index 35042a7c72..432a8c15cd 100644
--- a/erts/preloaded/src/prim_file.erl
+++ b/erts/preloaded/src/prim_file.erl
@@ -618,8 +618,10 @@ write_file_info_1(Filename, Info, TimeType) ->
         error:_ -> {error, badarg}
     end.
 
-set_owner(_EncodedName, undefined, undefined) ->
-    ok;
+set_owner(EncodedName, Uid, undefined) ->
+    set_owner(EncodedName, Uid, -1);
+set_owner(EncodedName, undefined, Gid) ->
+    set_owner(EncodedName, -1, Gid);
 set_owner(EncodedName, Uid, Gid) ->
     set_owner_nif(EncodedName, Uid, Gid).
 set_owner_nif(_Path, _Uid, _Gid) ->
diff --git a/lib/kernel/test/file_SUITE.erl b/lib/kernel/test/file_SUITE.erl
index 9a77454432..eea9e43dd3 100644
--- a/lib/kernel/test/file_SUITE.erl
+++ b/lib/kernel/test/file_SUITE.erl
@@ -56,7 +56,8 @@
 	  open1/1,
 	  old_modes/1, new_modes/1, path_open/1, open_errors/1]).
 -export([ file_info_basic_file/1, file_info_basic_directory/1,
-	  file_info_bad/1, file_info_times/1, file_write_file_info/1]).
+	  file_info_bad/1, file_info_times/1, file_write_file_info/1,
+          file_wfi_helpers/1]).
 -export([rename/1, access/1, truncate/1, datasync/1, sync/1,
 	 read_write/1, pread_write/1, append/1, exclusive/1]).
 -export([ e_delete/1, e_rename/1, e_make_dir/1, e_del_dir/1]).
@@ -152,7 +153,8 @@ groups() ->
      {pos, [], [pos1, pos2, pos3]},
      {file_info, [],
       [file_info_basic_file, file_info_basic_directory,
-       file_info_bad, file_info_times, file_write_file_info]},
+       file_info_bad, file_info_times, file_write_file_info,
+       file_wfi_helpers]},
      {consult, [], [consult1, path_consult]},
      {eval, [], [eval1, path_eval]},
      {script, [], [script1, path_script]},
@@ -1608,6 +1610,39 @@ file_write_file_info(Config) when is_list(Config) ->
     [] = flush(),
     ok.
 
+file_wfi_helpers(Config) when is_list(Config) ->
+    RootDir = get_good_directory(Config),
+    io:format("RootDir = ~p", [RootDir]),
+
+    Name = filename:join(RootDir,
+                         atom_to_list(?MODULE) ++ "_wfi_helpers"),
+
+    ok = ?FILE_MODULE:write_file(Name, "hello again"),
+    NewTime = {{1997, 02, 15}, {13, 18, 20}},
+    ok = ?FILE_MODULE:change_time(Name, NewTime, NewTime),
+
+    {ok, #file_info{atime=NewActAtime, mtime=NewTime}} =
+        ?FILE_MODULE:read_file_info(Name),
+
+    NewFilteredAtime = filter_atime(NewTime, Config),
+    NewFilteredAtime = filter_atime(NewActAtime, Config),
+
+    %% Make the file unwritable
+    ok = ?FILE_MODULE:change_mode(Name, 8#400),
+    {error, eacces} = ?FILE_MODULE:write_file(Name, "hello again"),
+
+    %% ... and writable again
+    ok = ?FILE_MODULE:change_mode(Name, 8#600),
+    ok = ?FILE_MODULE:write_file(Name, "hello again"),
+
+    %% We have no idea which users will work, so all we can do is to check
+    %% that it returns enoent instead of crashing.
+    {error, enoent} = ?FILE_MODULE:change_group("bogus file name", 0),
+    {error, enoent} = ?FILE_MODULE:change_owner("bogus file name", 0),
+
+    [] = flush(),
+    ok.
+
 %% Returns a directory on a file system that has correct file times.
 
 get_good_directory(Config) ->