use of static string in msTmpFile()

[sgillies@…]

In msTmpFile():    

    static char tmpId[128]; /* big enough for time + pid + ext */

I'm pretty sure that this doesn't need to be static.  Am going to make the change.
I appreciate comments or feedback.

[sgillies@…]

setting target to 4.6.

[dmorissette]

Agreed that it doesn't need to be static if you rework the function to
initialize tmpId on every call.

[dmorissette]

Sean,

I see in the code that you already removed the static in maptuil.c r1.178, but
you didn't adapt the code to properly initialize tmpId in all calls to the
function. As a result the msTmpFile() function is currently broken if it is
called more than once.

We'll fix this one, but if you removed other static instances elsewhere then
please review them or let us know where they are so that we can make sure that
the fixes won't have caused side-effects.

[sgillies@…]

Oops, sorry. I overlooked that we were depending on static-ness. I did not make
any other such changes.

[assefa]

I have updated the function to always initialize the tmpid (in both cvs head 
and 4.6 branch). It seems to work with my test data.
Marking it as Fixed. If any problems please reopen.

[fwarmerdam]

It seems various changes resulted in the counter always being set to 0, 
and to the forcebase logic not working at all.  I have updated the code
to restore these functionalities.  

Note that as the code was left, the counter was always zero, so two WMS 
requests in the same second would result in the same tmpfile name being
generated. 

I have only made this changes in 4.7 (cvs) so far, but it looks like it is
broken in 4.6 as well.   I am reopening so folks can review before I backport
the change into 4.6.

[fwarmerdam]

Current implementation:

char *msTmpFile(const char *mappath, const char *tmppath, const char *ext)
{
    char *tmpFname;
    char szPath[MS_MAXPATHLEN];
    const char *fullFname;
    char tmpId[128]; /* big enough for time + pid + ext */

    if( ForcedTmpBase != NULL )
    {
        strncpy( tmpId, ForcedTmpBase, sizeof(tmpId) );
    }
    else 
    {
        /* We'll use tmpId and tmpCount to generate unique filenames */
        sprintf(tmpId, "%ld%d",(long)time(NULL),(int)getpid());
    }

    if (ext == NULL)  ext = "";
    tmpFname = (char*)malloc(strlen(tmpId) + 4  + strlen(ext) + 1);

    sprintf(tmpFname, "%s%d.%s", tmpId, tmpCount++, ext);

    fullFname = msBuildPath3(szPath, mappath, tmppath, tmpFname);
    free(tmpFname);

    if (fullFname)
        return strdup(fullFname);

    return NULL;
}


Note, that I also change tmpCount to initialize to 0 instead of -1 
since we no longer need a "not initialized" marker. 

[assefa]

Frank,

 From what I can see it adresses the previous issue described in comment #3, 
as well as correcting the bug you described. So I see no reason not to port it 
to 4.6.1

Thanks for fixing it.

[fwarmerdam]

OK, it sounds like Sean is going to back port it. 

[sgillies@…]

Tests pass, and now it's in the 4.6 branch as well.

[szekerest]

But this problem exists in the current release (4.6.0), is it. I've run into 
the same problem: mapserver couldn't create the temp file, the file name was 
invalid because of the uninitialized tmpId on the second call.

[dmorissette]

Sean's fix was made after the 4.6.0 release and will be in 4.6.1. You can also
checkout CVS branch "branch-4-6" to get it.

Marking bug as FIXED (it was left with status REOPENED)

[cavallini@…]

When mapscript is loaded as dl it works, because pid is 
always different, and timestamp is correctly updated, but when it is loaded 
statically in php.ini it does not work, because timestamp is fixed at the time 
of mapscript loading and obviously pid is always the same; the counter does 
not resolve the ambiguity.

[fwarmerdam]

Paulo,

I don't follow your point.  The code uses a counter that increments for each
call to msTmpFile() so in the case of long running PHP instance all the temp
file will have the same <timestamp><pid> portion of the filename, but the
counter will increment keeping each of the requested msTmpFile() name requests
distinct. 

Have you actually seen a problem?  Can you see some reason why the 
incrementing counter doesn't make things unique?

The timestamp and pid stuff is intended to keep the files of different processes
distinct since they all start their counters from zero. 

[dmorissette]

Closing again. Please create new ticket with more specific details if there is really still a problem with this.