"Navin P" <[EMAIL PROTECTED]> writes:

> !     unsigned char *nm,tm[22],*km,*pm;

How did you get this number 22?

#   define ULLONG_MAX   18446744073709551615ULL

Here, ULLONG_MAX has 20 digits and '\0' is one more character, so
unsigned char tm[21] would have sufficed.  But long long might
have more than 64 bits, and so might RAND_MAX or time_t.

> !     unsigned long long t,k;

For portability, ELinks does not use long long unconditionally.
If you want to use long long, you should check whether
HAVE_LONG_LONG is defined, and do something else if not.
See the #define longlong in src/osdep/types.h.

> !     srand(time(NULL));
> !     t=rand()+ time(NULL) + 1LL;
> !     sprintf(tm,"%llu",t);

I do not think calling srand repeatedly is appropriate.

> !     /* This function already exists.We are using
> !      * some rand() as prefix in get_tempdir_filename
> !      * .This should fix the race condition even if
> !      * the file exists.The rand() is not necessarily
> !      * required but helps.
> !      * --navin */

rand() cannot fix the race condition, it can only make it more
difficult to reproduce.  Anyway if it were good to have a random
element in names of temporary files then I think that code should
be in get_unique_name rather than here, so that all callers could
benefit from it.

> !     nm=get_unique_name(pm);
> 
>       if (!init_string(&name)) {
> !             mem_free(nm);
> !             mem_free(pm);

get_unique_name apparently returns the original pointer if the
name is already unique.  Your code frees the same memory twice
in that case.

>       extension = get_extension_from_uri(uri);
>       if (extension) {

First get_unique_name (or tempnam) tries different names until it
finds one for which there is no file yet, and then get_temp_name
appends an extension, so the result of the check is no longer
valid.  Thus get_temp_name doesn't even notice clashes with files
that exist before it is called.  Is that why you needed rand()?

If get_unique_name is to be used then I think it should take the
extension as another parameter, so that it can append it before
checking whether the file already exists.  This might require
changing lookup_unique_name, too.

> -             /* FIXME: get_temp_name() calls tempnam(). --Zas */
>               file = get_temp_name(type_query->uri);

If this FIXME is removed, I think there should be a comment for
each variable and data member where the string is stored, warning
that another user may have created a file or symlink with that
name after the name was chosen.

Attachment: pgpBlAEeq41aC.pgp
Description: PGP signature

_______________________________________________
elinks-dev mailing list
[email protected]
http://linuxfromscratch.org/mailman/listinfo/elinks-dev

Reply via email to