Open asy972 opened 5 years ago
This already exists, and has existed since at least 2.3, but it's not very apparent unless you already know what to look for :(
If you arrange for "USE_SETPROCTITLE" to be defined during compilation, then your ps output will show information something like: setproctitle("%s: %s %s %s %s", servicename, clienthost, userid, mailbox, cmd);
. (Exactly what it will show will depend on which mechanisms your system makes available, and their limitations, but you get the idea.)
The best way to arrange for USE_SETPROCTITLE to be defined is to invoke configure like this before you compile:
./configure CFLAGS=-DUSE_SETPROCTITLE [your other configure args...]
(By putting the "CFLAGS=..." part as an argument to configure, it allows configure to record the fact that it was set. If you merely set CFLAGS in the environment and then run configure, it will have the same effect, but configure itself won't know it was there.)
I would be happy to review/merge a pull request to add an --enable-setproctitle
-style configure option!
his already exists, and has existed since at least 2.3, but it's not very apparent unless you already know what to look for :(
Thanks, it works!
I would be happy to review/merge a pull request to add an --enable-setproctitle-style configure option!
That would be good. But I would suggest that contrariwise: enable by default and add --disable-setproctitle-style option.
And yet there is a small bug (in 2.5.13): too many spaces at end of lines. ps ax | grep imap | sed "/ *$//" should be used for nice output.
And yet there is a small bug (in 2.5.13): too many spaces at end of lines. ps ax | grep imap | sed "/ *$//" should be used for nice output.
The patch (Linux, x86):
--- a/imap/setproctitle.c
+++ b/imap/setproctitle.c
@@ -276,6 +276,7 @@ setproctitle(const char *fmt, ...)
}
(void) strcpy(Argv[0], buf);
p = &Argv[0][i];
+ p++;
while (p < LastArgv)
*p++ = SPT_PADCHAR;
Argv[1] = NULL;
Thanks for the patch!
I'm reading this code and it looks like "fill the buffer with spaces" was a deliberate choice on the author's part.
Note that it deliberately starts writing SPT_PADCHAR (default: ' '
) at &Argv[0][i]
, overwriting the '\0'
added by strcpy and filling the entire Argv[0]
with spaces. And it doesn't add a '\0'
at the end of Argv[0]
, so I guess whatever uses this expects to use the full length, and doesn't expect a terminated c-string style. In absense of other bug reports over the years, I'm assuming this matches the behaviour expected by whatever uses it, but I don't know if or where that behaviour is documented so I can't confirm either way :(
Your patch preserves the '\0'
added by strcpy, but then still fills the rest of the buffer with SPT_PADCHAR. Anything expecting to use the whole length may be confused by the '\0'
in the middle, but anything expecting a terminated c-string would have definitely been confused by the unterminated buffer there previously. So I don't think this patch is safe to add, unless some documentation turns up showing that this can in fact be a terminated c-string vs a buffer-and-length.
But! It looks like we don't override SPT_PADCHAR if it's already defined, so even without the patch, you could still achieve the same effect (with the same risk) by pre-defining SPT_PADCHAR to '\0'
. In this case, instead of padding the buffer with spaces, it would pad it with '\0'
, and the string would end up terminated in the same way.
I guess you would do this with something like this, but I haven't tested and you might need to mess with the quotations to get the right value defined in the end...
./configure CFLAGS="-DUSE_SETPROCTITLE -DSPT_PADCHAR='\0'" [your other configure args...]
If we ever add a configure argument for enabling the setproctitle behaviour (rather than relying on -D in CFLAGS), I guess we could also add a corresponding one for selecting the padding character to use...
Note that it deliberately starts writing SPT_PADCHAR (default: ' ') at &Argv[0][i], overwriting the '\0' added by strcpy and filling the entire Argv[0] with spaces. And it doesn't add a '\0' at the end of Argv[0], so I guess whatever uses this expects to use the full length, and doesn't expect a terminated c-string style.
Yes, this is also an interesting point. I don't really understand why fill is needed. It works for me the same way (and it reduces the number of operations):
--- a/imap/setproctitle.c
+++ b/imap/setproctitle.c
@@ -275,9 +275,9 @@ setproctitle(const char *fmt, ...)
buf[i] = '\0';
}
(void) strcpy(Argv[0], buf);
- p = &Argv[0][i];
- while (p < LastArgv)
- *p++ = SPT_PADCHAR;
+// p = &Argv[0][i];
+// while (p < LastArgv)
+// *p++ = SPT_PADCHAR;
Argv[1] = NULL;
# endif
# endif /* SPT_TYPE != SPT_NONE */
How hard is it to find an author and ask his? Maybe it is necessary in not GNU/Linux OS (but some OS are processed separately as I see), or old Linux versions.
btw, is it known how many Cyrus-IMAP users use this option?
I found similar code in util-linux in lib/setproctitle.c:
i = strlen(buf);
if (i > argv_lth - 2) {
i = argv_lth - 2;
buf[i] = '\0';
}
memset(argv0[0], '\0', argv_lth); /* clear the memory area */
strcpy(argv0[0], buf);
argv0[1] = NULL;
Apparently '\0' can be used by default. And memset is more preferable than manual loop for for optimisation by compilator.
How hard is it to find an author and ask his? Maybe it is necessary in not GNU/Linux OS (but some OS are processed separately as I see), or old Linux versions.
The "padding with spaces" behaviour has been there since setproctitle was first added by deacf5d54 in 1994. There's an email address for the author in the commit, but who even knows if it's still valid, and even if it is, it was 25 years ago...
I found similar code in util-linux in lib/setproctitle.c:
Good find, that gives us some confidence in how it's supposed to work in Linux, at least! Looks very similar to what we've got minus the cross-platformness, so I wonder if our code was also "borrowed from sendmail" way back when
Hello.
It would be good to show logins and hostnames in the ps output. For example (Linux):