[parted-devel] Possible Race Condition using test code, libparted, and Fedora 12

[Petr Uzel]

Hi,

On Thu, Jan 28, 2010 at 09:11:41AM -0700, Curtis Gedak wrote:
> Petr Uzel wrote:
> >Don't you mean something like 'if (retry_count % 3 == 0)' instead?
> >That would pause on the 3rd, 6th and 9th (last) iteration.
> 
> My original patch did try to do something like you suggest.  After
> analyzing the patch, I realized that it only paused once in all of
> the 10 iterations.  This single pause was sufficient to catch all of
> the "failed to inform kernel of partition changes" problems that
> occurred during my testing.
> 
> Hence this time I did intend that the code only pause once for 1
> second.  :-)
>
> My thoughts are that 1 second is a very long time for a computer.  If
> more than an single 1 second pause is used, then in my opinion this
> would unnecessarily increase the amount of time required until the
> call would finally fail.  For example this function call would still
> fail when used on a device with at least one partition mounted.  I
> did not want this to creep up to 2 or 3 seconds without significant
> benefits to the user.

Yes, this make sense. But what confused me (and made me assume your
intention was to sleep 3 times) is why it should sleep on 7th
iteration out of 10. I don't think that after 'unsuccessful' one
second sleep in 7th iteration the two other non-sleeping iterations
would make any difference. Wouldn't be better if it sleeps on the last
iteration instead? Or even get rid of the 9 non-sleeping iterations
and just retry the ioctl after one second sleep and then give up?

I know, this is nitpicking, but it simply isn't clear to me why
the '3' is there. 
 

Regards,

Petr

--
Petr Uzel, openSUSE Boosters Team
IRC: ptr_uzl @ freenode
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/parted-devel/attachments/20100129/e2e7d23c/attachment.pgp>