[Ocfs2-tools-devel] corrupt_extent5.patch of modification with Sunil

[tao.ma]

Here is the new patch.
Sunil Mushran wrote:

> 1. When submitting a newer patch, increment the number so that there
> is no confusion. I have multiple "corrupt_extent.patch" in my mbox. :)
> corrupt_extent4.patch would have been clearer.

Sorry. This time I make the new name corrupt_extent5.patch.

>
> 2. Use mktemp(3) instead of /dev/urandom to generate a random filename.
> Even if you were to use the latter, you will need to convert the binary
> to ascii (hex, maybe).

Have modified it to mktemp.

>
> 3. line 761:
> +               n_clusters = 1;
> Is this necessary?

Copied it from ocfs2_extend_allocation. Since it is no use, remove it.

>
> 4. FSWRK_FATAL should be used with care. It should be reserved
> for cases for which we don't expect to encounter, like failed malloc,
> and thus do not want to spend time coding the handling of the error.
> In create_tmp_directory() you are handling the error condition but also
> calling FATAL. I would rather that you handle the error itself.
> Also, if you need to use it, make sure the error is not ambiguous.
> For e.g., in ocfs2_extend_allocation_specific(), if one were to encounter
> the error, it would not be immediately clear whether it was due to a 
> failed
> alloc or a failed insert. BTW, failed disk allocs should be handled 
> and not
> thrown to FATAL.

Remove many call of FSWRK_FATAL and add my own error handling.

>
> 5. extend_file_size() does not look correct. You read_cached_inode()
> twice yet free once. Memory leak.

  Add another free calling. Thx.

>
> 6. Reuse existing types. For e.g., OCFS2_FT_DIR denotes a dir. The type
> in extend_file_size introduces a new one.
>
> 7. See ocfs2_expand_dir() wrt the same function above. As in, I don't 
> mind
> if extend_file() just works on regular files and ignores dirs.

Sorry, I think I use the wrong word of "type". So rewrite the function 
and changed it to name "flag" and add two macros for code's readability.

>
> 8. Coding style. Always have a line separating the variable 
> declaration section
> and the code.

Recheck the whole patch and add them. Thx.

>
> 9. Don't add Authors line in the copyright. We now add the author in
> CREDITS rather than in each individual file. Year should be 2006.

Remove them.


-- 
* **     Tao Ma
*     Member of Techincal Staff *

Oracle Asia Development Center
Emerging Technology & Solution Development
*
Tel:        +86 10 8278 6026  
URL:       OADC Intranet <http://cdc.oraclecorp.com/>, Oracle.com/cdc 
<http://www.oracle.com/cdc/>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: corrupt_extent5.patch
Type: text/x-patch
Size: 23039 bytes
Desc: not available
Url : http://oss.oracle.com/pipermail/ocfs2-tools-devel/attachments/20060901/9526cea5/corrupt_extent5.bin