[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