Re: [Rpm-maint] Plugin ponderings
Thank you for all clarifications! Now I am attaching the corrected patch and
the most important is that I understood the indentation finally (took a
while through :()!
Hopefully this is the last iteration for the script cases :)
-Original Message-
From: Panu Matilainen [mailto:pmati...@laiskiainen.org]
Sent: Thursday, November 29, 2012 2:38 PM
To: Reshetova, Elena
Cc: rpm-maint@lists.rpm.org
Subject: Re: Plugin ponderings
On 11/29/2012 01:52 PM, Reshetova, Elena wrote:
Yes, I am sorry: I have made two mistakes now:
1) patch is wrong (sent the intermediate version before the
compilation and plenty of other changes)
Ok, thought that might be the case.
2) and yes, I had default indentation of 4, which was confusing me for
a long long while :( I was just using simple Ubuntu file editing, I
changed it now and it looks much better.
Heh. That certainly explains a lot, you must've been thinking I'm crazy
complaining about indentation on what looks like the most horrid
misindentation-mess you've ever seen :D
The rpm codebase does look like the indentation level is 4, because it
actually is. It's just that its not 4 spaces or tabs of 4, but a combination
of zero or more hardtabs of width 8, plus one softtab (4
spaces) where needed to place things between the hard tabs.
Just in case, here's a practical (if extremely stupid :) example with the
indentation levels spelled out below:
int foo(int x)
{
int rc = 0;
for (int i = 0; i x; i++) {
rc++;
if (rc 255) {
if (rc 65535) {
printf(WAY too big!);
} else {
printf(too big!);
}
}
}
return rc;
}
0 hardtabs, 1 softtab
1 hardtab, 0 softtabs
1 hardtab, 1 softtab
^ 2 hardtabs, 0 softtabs
I will send a new version in a sec with things corrected.
Sorry again for all of this!
Mistakes happen, no worries. I'm just glad we identified the source of
indentation issues - the possibility (and consequences!) of hardtab set to
width of four only occurred to me after seeing these last two patches.
For the future, if it seems like I'm talking complete nonsense, please just
say so. It's entirely possible that
a) I actually am talking complete nonsense (like with the fsm commit stuff,
still puzzling how I could remember it *that* wrong)
b) There's some local difference causing you to see something entirely
different than I do. Be it actual functionality or editor settings :)
...etc.
- Panu -
0001-Improving-scriptlet-related-rpm-plugin-hooks.patch
Description: Binary data
smime.p7s
Description: S/MIME cryptographic signature
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] Plugin ponderings
On 11/29/2012 03:00 PM, Reshetova, Elena wrote:
Thank you for all clarifications! Now I am attaching the corrected patch and
the most important is that I understood the indentation finally (took a
while through :()!
Much better late than never, the indentation is now exactly as it should
be, good! There's just one extra leftover indent level for the execv() call.
Hopefully this is the last iteration for the script cases :)
I'm afraid we'll need one more round: you perhaps took my that's what
RPMSCRIPTLET_EXEC bit is for (wrt embedded scripts) a bit too literally :)
What I meant is that the RPMSCRIPTLET_EXEC bit defines whether something
is embedded or external - external scripts always have it, embedded ones
never have it as there is nothing to exec. The patch gets it a bit
backwards in rpmScriptRun():
if (rstreq(args[0], lua))
script_type = RPMSCRIPTLET_EXEC;
^^
That should be 0 currently as we're not forking nor execing anything for
lua scriptlets, and EXEC it can *never* have. Then the rest becomes
simply something like:
if (rc != RPMRC_FAIL) {
if (script_type RPMSCRIPTLET_EXEC) {
rc = runExtScript(plugins, selinux, prefixes,
script-descr, lvl, scriptFd, args, script-body, arg1, arg2);
} else {
rc = runLuaScript(selinux, prefixes, script-descr, lvl,
scriptFd, args, script-body, arg1, arg2);
}
}
While not strictly necessary, I think rpmLuaScript() could be changed to
take the additional 'plugins' argument while we're at it, if only to
keep symmetry between the two functions. It'll be needed later on
anyway, unlike the selinux argument which gets passed to runLuaScript()
just for the symmetry's sake.
- Panu -
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] Plugin ponderings
Much better late than never, the indentation is now exactly as it should be, good! There's just one extra leftover indent level for the execv() call. Oh, missed that one since it wasn't in diff: fixed now! I'm afraid we'll need one more round: you perhaps took my that's what RPMSCRIPTLET_EXEC bit is for (wrt embedded scripts) a bit too literally :) What I meant is that the RPMSCRIPTLET_EXEC bit defines whether something is embedded or external - external scripts always have it, embedded ones never have it as there is nothing to exec. The patch gets it a bit backwards in rpmScriptRun(): Oh, then I indeed misunderstood you: I was thinking that external scripts will have fork + exec and lua ones just exec, since they are executed after all (just not via exec). Now fixed, too. While not strictly necessary, I think rpmLuaScript() could be changed to take the additional 'plugins' argument while we're at it, if only to keep symmetry between the two functions. It'll be needed later on anyway, unlike the selinux argument which gets passed to runLuaScript() just for the symmetry's sake. Sure, added! I am attaching the new version now *without* saying that this is the last one :) Best Regards, Elena. 0001-Improving-scriptlet-related-rpm-plugin-hooks.patch Description: Binary data smime.p7s Description: S/MIME cryptographic signature ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] Plugin ponderings
On 11/29/2012 04:57 PM, Reshetova, Elena wrote: Much better late than never, the indentation is now exactly as it should be, good! There's just one extra leftover indent level for the execv() call. Oh, missed that one since it wasn't in diff: fixed now! I'm afraid we'll need one more round: you perhaps took my that's what RPMSCRIPTLET_EXEC bit is for (wrt embedded scripts) a bit too literally :) What I meant is that the RPMSCRIPTLET_EXEC bit defines whether something is embedded or external - external scripts always have it, embedded ones never have it as there is nothing to exec. The patch gets it a bit backwards in rpmScriptRun(): Oh, then I indeed misunderstood you: I was thinking that external scripts will have fork + exec and lua ones just exec, since they are executed after all (just not via exec). Now fixed, too. While not strictly necessary, I think rpmLuaScript() could be changed to take the additional 'plugins' argument while we're at it, if only to keep symmetry between the two functions. It'll be needed later on anyway, unlike the selinux argument which gets passed to runLuaScript() just for the symmetry's sake. Sure, added! I am attaching the new version now *without* saying that this is the last one :) Heh, curse of the famous last words avoidance :) It worked too - applied pushed now. Thank you! - Panu - ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
