Related issue is ZBXNEXT-1855, a few useful links there.

When a process starts we have the following situation:

argv[0] string\0argv[1] string\0...argv[argc-1] string\0          <--- This buffer is accessible both internally and externally (via /proc/pid/cmdline on Linux)
^               ^                  ^
argv[0]         argv[1]            argv[argc-1]          argv[argc] -> NULL          <--- This array is accessible only internally

As long as we guarantee that we have read command line parameters before we use setproctitle() and we don't try to access them afterwards there is no difference where argv[0<i<argc] will point since they cannot be accessed externally. Setting them to NULL or making them point to an empty string is double proofing that we do not read rubbish we put through setproctitle() ourselves as valid command line parameters. Latter would have probably been safer if we had used standard library functions to operate with strings (some of them are not NULL-instead-of-valid-char*-proof).

The safest option:

static char	empty_str[] = "";

Simply silencing compiler (almost as safe in our situation and least likely to break something):

static char	*empty_str = NULL;

Throwing out argv[i] overwriting altogether wont break anything now, but may lead to weird and very hard to find bugs in the future.

In my first comment I made a few false statements therefore I should clarify how our setproctitle.c works.

Code in question is active on Linux, AIX, Solaris and OS X. First of all in server, proxy and agentd main() we call setproctitle_save_env():

int	main(int argc, char **argv)
{
/* variable declarations */

#if defined(PS_OVERWRITE_ARGV) || defined(PS_PSTAT_ARGV)
	argv = setproctitle_save_env(argc, argv);
#endif

setproctitle_save_env() allocates new arrays arr_int and environ_int, makes copies of command line arguments and environment variables which may get overwritten, fills argv_int and environ_int with pointers to copies of original parameters (or to original parameters which cannot get overwritten), prepares buffer where we will later put status messages, saves environ as environ_ext, assigns environ_int to environ and returns argv_int (which will be immediately assigned to argv as we see above). Pointer to old argv array is therefore lost.

In the end of server, proxy and agentd main() we call setproctitle_free_env:

#if defined(PS_OVERWRITE_ARGV)
	setproctitle_free_env();
#endif

	exit(SUCCEED);
}

setproctitle_free_env() frees all the memory allocated by setproctitle_save_env() and restores original value of environ variable. Original value of argv is not restored.

In between we call setproctitle_set_status() which overwrites buffer prepared by setproctitle_save_env() with desired status message.

As I said, access to the old argv is lost and wont be restored. Some of its elements are set by setproctitle_save_env() to point to an empty string (in a way that causes a warning), some may still point to original arguments, but if we could access (old)argv[argc_ext_copied_first] we would read all the garbage we put with setproctitle_set_status() instead of original argument. Apparently, it causes no problem. Hence we can safely omit putting empty string plugs.

More improvements worth considering:

• don't allocate argv_int, simply overwrite argv (this won't change behaviour);
• treat argv and environ equally, either overwrite or restore both.

First of all, desired output of ps for Zabbix daemons should look like:

command parameters     <--- for parent process
command: status_message     <--- for child processes

Currently on Solaris output differs:

command parameters     <--- for parent process
command parameters: status_message     <--- for child processes

It was done this way probably because ps on Solaris seems to update command and parameter string of a process only if it gets longer (otherwide it falls back to original command). However, we can try to pad short status messages with '-' or '=' (unfortunately spaces don't work) so that they are at least the same length as original command. This will look like this:

command parameter_list     <--- for parent process
command: long_status_message     <--- for child processes
command: short_status=     <--- for child processes

Under the hood situation is following:

argv[0] string\0argv[1] string\0...argv[argc-1] string\0          <--- this where ps will search for command and parameters on Linux (and likely OS X)
^               ^                  ^
argv[0]         argv[1]            argv[argc-1]          argv[argc] -> NULL          <--- this is where ps looks on Solaris (and probably on AIX as well)

On Linux we need to make our internal copy of every argv[i] string and environment variable to free up buffer for status messages, but we can overwrite argv with new pointers because ps won't look into it.

On Solaris we need to make our internal copy of argv pointers to keep original argv for ps, allocate buffer for status messages, link in to original argv[0], set argv[0<i<argc] = "" (or maybe simply argv[1] = NULL) and add padding.

Since approaches are very different I would split the code into several #ifdefs for easier maintenance like it has been already done for HP-UX code.

Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-9863-22 revision 55941. Fix is very minimalistic and is as safe as possible.

More radical experiments on improvement of setproctitle() are in the same development branch before r55939.

(1) while the current fix is working, I think the one suggested initially would be more straightforward:

static char     *empty_str = "";

We can discuss it with asaveljevs when he will return.

wiper So the current vote is for static char *empty_str = "";

glebs.ivanovskis Renamed variable back to empty_str in r55965, suggested minor improvements in r55968. RESOLVED

wiper CLOSED

Successfully tested

Fixed in pre-2.2.11rc1 r55975.
Fixed in pre-2.4.7rc1 r55976.
Fixed in pre-3.0.0alpha3 r55977.