[Mono-dev] [PATCH]System.IO.File.Replace

Rodrigo Kumpera kumpera at gmail.com
Wed Aug 15 18:42:08 EDT 2007


Hi Bill,

Below are some comments regarding your patch:

+
+    return(ret);
+}

Don't use parentesis around the return value, it makes things harder to
read.

+    int remain, n;
+    int buf_size = st_src->st_blksize;
+    char *buf = (char *) alloca (buf_size);
+

It's better to avoid alloca for quite a few reasons and on this case, it's
better to limit the maximum size of buf_size since it's value is outside
mono's control.

-
-    g_free (utf8_src);
-    g_free (utf8_dest);
-    close (src_fd);
-    close (dest_fd);
-
+
+    g_free (utf8_replacedFileName);
+    g_free (utf8_replacementFileName);
+    g_free (utf8_backupFileName);
+    close (backup_fd);
+

Notice here that you're introducing some redudant whitespaces in the empty
lines, this should be avoided as it make patches hard to read due to
increased noise.

Besides that, I have a question. Wouldn't be possible to use sendfile for
unix systems? Senfile skip copying data to user space and should be a lot
faster than using read/write.

Cheers!
Rodrigo


On 8/15/07, Bill Holmes <billholmes54 at gmail.com> wrote:
>
> Attached is a patch that implements the 2.0 method File.IO.File.Replace.
>
> http://msdn2.microsoft.com/en-us/library/9etk7xw2.aspx
>
> Some implementation details are as follows.
> -An internal call was added to perform this operation
> -Most argument validation is performed in managed code.
> -On windows we call ReplaceFile defined in Kernel32
> -On !windows Replace file was implemented in io-layer/io.c
>
> *file-io.c: Added ves_icall_System_IO_MonoIO_ReplaceFile to call
> ReplaceFile Kernel32 on windows or in io-layer.
> *file-io.h: Added deceleration for ves_icall_System_IO_MonoIO_ReplaceFile
> *icall-def.h: Register ves_icall_System_IO_MonoIO_ReplaceFile as an
> internal call.
> *io.c: Added implementation for ReplaceFile.
> *io.h: Added deceleration for ReplaceFile method.
> *File.cs:  Add implementation for IO.File.Replace methods.
> *MonoIO.cs: Declared an internal call for ReplaceFile
> *FileTest.cs:  Added a test for IO.File.Replace.
>
> This may be less of a patch request and more of a "Tell me what I did
> wrong."  ;)
>
> -bill
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20070815/b4e1b554/attachment.html 


More information about the Mono-devel-list mailing list