httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Richards <p.richa...@elsevier.co.uk>
Subject Re: Replacement for mktemp and httpd_monitor
Date Fri, 22 Dec 1995 11:28:11 GMT
In reply to Jim Jagielski who said
> 
> Because Apache currently uses mktemp(), which has a format that varies from
> OS to OS, it's beneficial for it to use a version in which _it_ defines
> the format. This allows for the use of a "universal" httpd_monitor
> app, for example.
> 
> The diffs basically do a few things:
>    o define a new format for mkatemp() files ("filename.xxxxxxxx") This
>      is because we assume 4 bytes for a long (which getpid() is cast
>      to when the PidFile is printed) but we also don't worry too much
>      about the size since we only allow up to 8 fields.
>    o the name of the scoreboard file now uses the PID in hex, rather
>      than decimal. Again, it's limited to 8 hex chars.
>    o httpd_monitor now reads the PID directly from the PidFile and
>      emulates mkatemp internally, so that it'll always grab the right
>      scoreboard file (since it uses the valid PID and forces the scoreboard
>      name just like mkatemp()).

There's some over-engineering taking place here and some decisions I
don't like. I want to be able to see the assosciated process id at a glance
and that's hard if it's in hex. http_monitor has to have the same mkatemp as
Apache does, this isn't consistent with using a scoreboard name that
arbitrary third party tools can access.

Here's an example of what should be done with some
general programing changes thrown in for good measure.

Strings should be malloced at run time, avoids problems with
fixed length buffers and reduces the executable image and generally make
better use of memory. Also makes security holes with sprintf a non-issue
because strings aren't on the stack. It'd also make string overflows
rather obvious because the thing will SIGSEGV.

void
accept_mutex_init(pool *p)
{

/* This would be picked up from a config file */
#define LOCK_FILE "/some/path/lock"
 
	char *file_fname; /* Probably  need sto be global */
	pid_t pid;

	pid = getpid();

	len = strlen(LOCK_FILE);

	lock_fname = malloc(len + 8);

	if (!lock_fname) 
		/* Do something like die */
	
	sprintf(lock_fname, "%s.%8d", LOCK_FILE, pid);

    lock_fd = popenf(p, lock_fname, O_CREAT | O_WRONLY, 0644);
    if (lock_fd == -1)
    {
        perror ("open");
        fprintf (stderr, "Cannot open lcok file\n");
                                      ^^^^
                                    Easy bug fix for someone :-)

        exit (1);
    }
------>    unlink(lock_fname);

	What's the purpose of this?
}

Similar things should be done elsewhere in Apache too.
 
Why is there a need for a lock file? It looks to me like there's a file
being created which then has fcntl applied to it as a locking mechanism.
If fcntl works across platforms and is used in this manner, why not lock
the scoreboard file itself. If fcntl isn't portable (I'm not sure it
is actually) then the simple existence of the lock file is enough for
locking and fcntl on it is redundant. fcntl is likely better performance
wise than creating and deleting the lock file though so I'd accept it
on that basis if it truly is portable.

-- 
  Paul Richards. Originative Solutions Ltd.
  Internet: paul@netcraft.co.uk, http://www.netcraft.co.uk
  Phone: 0370 462071 (Mobile), +44 1225 447500 (work)

Mime
View raw message