[Ocfs2-tools-devel] corrupt_extent6.patch of fswreck

[tao.ma]

Thanks for your advice.

Sunil Mushran wrote:

> 1. Please don't do this:
> +errcode_t create_tmp_directory(ocfs2_filesys *fs,
> +                               char *dirname, uint64_t *blkno)
> +{
> +       errcode_t ret = ocfs2_new_inode(fs, blkno, S_IFDIR | 0755);
> +
> +       if (ret)
> +               goto new_inode_fail;
>
> Use the normal style.
>
> +errcode_t create_tmp_directory(ocfs2_filesys *fs,
> +                               char *dirname, uint64_t *blkno)
> +{
> +       errcode_t ret;
> +
> +       ret = ocfs2_new_inode(fs, blkno, S_IFDIR | 0755);
> +       if (ret)
> +               goto new_inode_fail;

done.

>
> 2. Use OCFS2_MAX_FILENAME_LEN.
> +       char parentdir[256];

done.

>
> 3. In ocfs2_extend_allocation_specific() the for loop is unorthodox.
> Not incorrect but makes it unnecessarily unreadable.
> Instead of:
> +               for(i = n_clusters - 1;; i--){
> +                       if (i == 0)
> +                               break;
> Do:
> +               for(i = n_clusters; i; --i){
>
> The latter is lot more readable. Adjust "i" in the loop.

done.

>
> 4. In the same function, try not to do computation in each loop.
> Use a variable to make life easier. While this is not much of an issue
> in a tool, it becomes when you do the same in the module. Better you
> learn to start thinking in terms of cpu usage now. :)
> >> blkno + i * blk_per_cluster
> Redo inorder to eliminate the multiply.

done.

>
> 5. Try not to name two functions literally the same. For e.g.,
> damage_ocfs2_extent_blocks() and damage_ocfs2_extent_block().
> I understand the reasoning but it makes for poor readability.
> How about, damage_one_extent_block and damage_many_extent_blocks.

done.

>
> 6. Overall try to provide helpful error messages where it happens 
> rather than
> in consolidated places. This is because it is easier to pinpoint the 
> error location.
> While this is not a core requirement for fswreck, it is for any end 
> user tool.
> (We are doing so because we've had a hard time diagnozing errors 
> encountered
> at ct sites.)

Add some help informations.

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

Oracle Asia Development Center
Emerging Technology & Solution Development
*
Tel:        +86 10 8278 6026
Mobile:   +86 13701237602         
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_extent6.patch
Type: text/x-patch
Size: 23364 bytes
Desc: not available
Url : http://oss.oracle.com/pipermail/ocfs2-tools-devel/attachments/20060904/e53b2e52/corrupt_extent6-0001.bin