Re: 1.11.22 and MSVC

[Mark D. Baushke]

Sergei Antonov <saproj@gmail.com> writes:

> Hello!
> I've found several bugs in cvs-1.11.22 under Windows using MSVC 6.0 and 8.0.

Thank you very much for your report.

> 
> BUG #1
> line
>     free (cvs_password);
> in src/login.c
> and line
>  free (password);
> in src/client.c
> 
> both free the same buffer, so, when the "login" command is being executed,
> "free (cvs_password);" tries to free an already freed pointer. Debug runtime
> checks in MSVC catch this.

Hmmm... on the other hand, there are other code paths that need the
password returned by get_cvs_password to be freed. See the patch below.
I don't have time to test this right now, so if you could rerun your
test to see if it works, that would be great.

> 
> BUG #2
> cvs.exe compiled for 64-bit Windows cannot set correct filetime during the
> checkout command. The most correct way to fix this is to add preprocessor
> definition HAVE_SYS_UTIME_H to all relevant .dsp files. MSVC (at leat
> 6.0and later) does have sys/utime.h.

I'll have to let our windows-NT maintainers deal with this one. I do not
know which MSVC revision they are intending to use as we move forward.

> BUG #3
> All .dsw and .dsp files must have \r\n line ends instead of \n line ends.
> Without that Visual Studio doesn't open them.

They probably started life this way and had the line endings changed by
checkout from a UNIX box because the files are not -kb in nature. If you
checkout a copy of the cvs1-11-x-branch directly, I suspect things
should work.

> I also tried to compile the latest ccvs from the repository using MSVC 8.0.
> I didn't succeed but found two errors in windows-NT/dirent.c in lines
>   dirp = xmalloc (sizeof DIR);
> and
>       dp = xmalloc (sizeof struct _dircontents);
> According to C standard, sizeof operand which is a type must be
> parenthesized.

Hmmm... Yes.

In C89, section '6.3.3.4 The sizeof operator' we see this:

...
The sizeof operator results in a
value that is the size, in bytes, of the variable or type that it
precedes. This operator has two forms. If it is applied to a type, then
sizeof takes this form:

    sizeof(type)

That is, the type name must be enclosed within parenthesis. If sizeof is 
applied to a variable, you may use this form:

    sizeof var_name

In this case, no parenthesis are necessary.
...

So, I have committed a fix for this in the FEATURE branch (currently
top-of-tree of the repository). If you update your tree, you should find
that it will no longer have this problem.

        Enjoy!
        -- Mark

Untested fix for duplicate free() problem. It would be good to know
which code path you were exercising to get the duplicate free() so that
a test may be added to the sanity.sh test cases.

--- cvs-1.11.22/src/client.c~   Thu Jun  8 07:36:03 2006
+++ cvs-1.11.22/src/client.c    Sun Jul 23 09:17:01 2006
@@ -3919,9 +3919,8 @@ auth_server (root, lto_server, lfrom_ser
        send_to_server(end, 0);
        send_to_server("\012", 1);
 
-        /* Paranoia. */
-        memset (password, 0, strlen (password));
-       free (password);
+       free_cvs_password (password);
+       password = NULL;
 # else /* ! AUTH_CLIENT_SUPPORT */
        error (1, 0, "INTERNAL ERROR: This client does not support pserver 
authentication");
 # endif /* AUTH_CLIENT_SUPPORT */
--- cvs-1.11.22/src/cvs.h~      Mon May 15 17:59:21 2006
+++ cvs-1.11.22/src/cvs.h       Sun Jul 23 09:38:24 2006
@@ -916,6 +916,7 @@ char *descramble PROTO ((char *str));
 
 #ifdef AUTH_CLIENT_SUPPORT
 char *get_cvs_password PROTO((void));
+void free_cvs_password PROTO((char *str));
 int get_cvs_port_number PROTO((const cvsroot_t *root));
 char *normalize_cvsroot PROTO((const cvsroot_t *root));
 #endif /* AUTH_CLIENT_SUPPORT */
--- cvs-1.11.22/src/login.c~    Thu May  4 08:13:41 2006
+++ cvs-1.11.22/src/login.c     Sun Jul 23 09:42:49 2006
@@ -566,17 +566,30 @@ login (argc, argv)
     password_entry_operation (password_entry_add, current_parsed_root,
                               typed_password);
 
-    memset (typed_password, 0, strlen (typed_password));
-    free (typed_password);
-
-    free (cvs_password);
+    free_cvs_password (typed_password);
     free (cvsroot_canonical);
-    cvs_password = NULL;
 
     return 0;
 }
 
 
+
+void
+free_cvs_password (char *password)
+{
+    if (password && password != cvs_password)
+    {
+       memset (password, 0, strlen (password));
+       free (password);
+    }
+
+    if (cvs_password)
+    {
+       memset (cvs_password, 0, strlen (cvs_password));
+       free (cvs_password);
+       cvs_password = NULL;
+    }
+}
 
 /* Returns the _scrambled_ password.  The server must descramble
    before hashing and comparing.  If password file not found, or