[Ocfs2-tools-devel] [PATCH 06/10] Dir operation modification for inline data.

[Joel Becker]

On Sat, Jul 12, 2008 at 10:13:07AM +0800, Tao Ma wrote:
> Joel Becker wrote:
>> On Fri, Jul 11, 2008 at 02:20:59PM +0800, Tao Ma wrote:
>>> +	ocfs2_dinode_new_extent_list(fs, di);
>>> +
>>> +	di->i_size = 0;
>>> +	di->i_ctime = di->i_mtime = time(NULL);
>>> +
>>> +	ret = ocfs2_write_inode(fs, dir, di_buf);
>>
>> 	This is a little weird, writing out a zero size, even
>> temporarily.  If we crash, we lose the dir entries, right?
> yes, you see ocfs2_insert_extent below needs us to send the blkno and  
> read the inode info by itself, so I have to write it out here. by now, I  
> haven't find a perfect way to handle this(no journal in user space is  
> really painful).

	I know why you did it, I just don't like the consequences.
ocfs2_insert_extent() can't handle an i_size that isn't 0?

>>> @@ -128,15 +129,24 @@ errcode_t ocfs2_link(ocfs2_filesys *fs, uint64_t dir, const char *name,
>>>  	ls.flags = flags;
>>>  	ls.done = 0;
>>>  	ls.sb = fs->fs_super;
>>> -
>>> +again:
>>>  	retval = ocfs2_dir_iterate(fs, dir,
>>>                                     OCFS2_DIRENT_FLAG_INCLUDE_EMPTY,
>>>                                     NULL, link_proc, &ls);
>>>  	if (retval)
>>>  		return retval;
>>>  -	if (!ls.done)
>>> -		return OCFS2_ET_DIR_NO_SPACE;
>>> +	if (!ls.done) {
>>> +		if (allocation)
>>> +			return OCFS2_ET_INTERNAL_FAILURE;
>>> +
>>> +		retval = ocfs2_expand_dir(fs, dir);
>>> +		if (retval)
>>> +			return retval;
>>> +
>>> +		allocation = 1;
>>> +		goto again;
>>> +	}
>>
>> 	Can we make this a loop instead of a goto?  It's not complex.
> en, not agree, if we use loop, use "for" or "while"?
> if with "for", then use "for(i=0;i<2;i++)" isn't looks as clear as this  
> one. the same goes with "whle(i<2)".

	Sure, but you don't even need to loop - you can just code it
like we used to in other functions.  Saving 3 whole lines of code isn't
worth gotos.  Here's a few ways to do it:

[Loop]
	do {
 		retval = ocfs2_dir_iterate(fs, dir,
					   OCFS2_DIRENT_FLAG_INCLUDE_EMPTY,
					   NULL, link_proc, &ls);
		if (retval)
			return retval;
		if (ls.done)
			break;
		if (allocation)
			return OCFS2_ET_INTERNAL_FAILURE;

		retval = ocfs2_expand_dir(fs, dir);
		if (retval)
			return retval;

		allocation = 1;
	} while (1);

[No loop at all]
 	retval = ocfs2_dir_iterate(fs, dir,
                                   OCFS2_DIRENT_FLAG_INCLUDE_EMPTY,
                                   NULL, link_proc, &ls);
	if (retval)
		return retval;
	if (!ls.done) {
		retval = ocfs2_expand_dir(fs, dir);
		if (retval)
			return retval;

		retval = ocfs2_dir_iterate(fs, dir,
					   OCFS2_DIRENT_FLAG_INCLUDE_EMPTY,
					   NULL, link_proc, &ls);
		if (retval)
			return retval;
		if (!ls.done)
			return OCFS2_ET_INTERNAL_FAILURE;
	}

Joel

-- 

Life's Little Instruction Book #456

	"Send your loved one flowers.  Think of a reason later."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127