On 01/28/2013 02:47 PM, Reshetova, Elena wrote:
Hi,
I am attaching the next version of the patch with modifications explained
inline below.
The patch looks deceptively simple :) In principle things look ok to me,
the issues I see have mostly to do with the symmetry part:
In fsmMkdirs(), the FileOpen() hook is not called, only
rpmpluginsCallFsmFileClose(). If we promise symmetry, we need to stick to it
here too. Looks like just an oversight and trivial to fix.
Yes, fixed now: I was looking to install/remove mostly and forgot about
symmetry on this one.
BTW, just spotted two other (new) issues regarding fsmMkdirs() in this
version of the patch: passing fsm->action to fsmMkdirs() and from there
to plugin hooks is not right, its probably FA_UNKNOWN at that state.
Considering what fsmMkdirs() does, the right thing to do is passing an
explicit FA_CREATE to the hooks.
The other issue is a different kind of symmetry issue:
+
+ /* Run fsm file open hook for all plugins */
+ rc = rpmpluginsCallFsmFileOpen(plugins, dn, mode,
DIR_TYPE_UNOWNED, action);
+ if (rc)
+ break;
+
rc = fsmMkdir(dn, mode);
+
+ /* Run fsm file closed hook for all plugins */
+ rpmpluginsCallFsmFileClose(plugins, dn, mode,
DIR_TYPE_UNOWNED, action, rc);
If the FileOpen() hook returns an error, this will bail out of the loop
before FileClose() gets called. I'd think it'd be sufficient to do just
rc = rpmpluginsCallFsmFileOpen(...);
if (!rc)
rc = fsmMkdir(dn, mode);
rpmpluginsCallFsmFileClose(...);
...as it'll break out of the loop on non-zero rc in the next step anyway.
- Panu -
- Panu -
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
