[Rpm-maint] FSM hooks for rpm plugin

Reshetova, Elena elena.reshetova at intel.com
Mon Feb 4 12:18:06 UTC 2013


>I've started to wonder about the hook names though: "open" and "close"
>seem out of place especially here (and to lesser degree elsewhere).
>Perhaps these hooks should simply follow the pre/post pattern even in the 
>names, ie FILE_PRE and FILE_POST. I think it'd be more descriptive wrt what 
>they actually do, and leave open and close free for possible later use for 
>the cases where a kind of open+close (files from payload
>stream) is actually occurring.

Sure, this is an easy change and I just did it. But the one below has made me 
a headache :(

>This gets quite ugly indeed, better refactor the surrounding code (which is 
>ugly in itself) to allow for nice clean pairing.  Oh and btw, this version of 
>the patch introduces a new asymmetry case: if the open-hook fails, close-hook 
>wont get called. It'll need something like open-hook just flagging postpone 
> >without terminating the loop, and adjusting the rest of the code to deal 
>with it. I'll have a look at this, how hard can it be ;)

I have written and rewritten now this part and still it looks ugly or even 
worse :( Flagging postpone is actually easy as well as not breaking out of 
loop in the beginning, but the early return is a small nightmare. If you 
introduce a variable tracking this, smth like "hardbreak" and setting it at 
that point, then you have a number of stupid if() causes, which make the whole 
constructions completely unreadable in addition to handling postpone and rc 
status (I have to walk thought this with pen and big white sheet to track the 
cases). Other option is "goto xyz" to bypass this.... but I am not sure this 
is any better, but might be easier to read. What do you think is better?

I have also fixed the other two issues you pointed in the other email.

Best Regards,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 7220 bytes
Desc: not available
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20130204/13bbe6a7/attachment.p7s>

More information about the Rpm-maint mailing list