Re: [Cvs-dev] cvs-passwd patch: weird problem

[Mark D. Baushke]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Prasad J Pandit <address@hidden> writes:

>    Hello Mark :)
> 
> On Sat, 19 Aug 2006, Mark D. Baushke wrote:
> > What output do you get with the following commands on your FC5 system:
> >
> >     uname -a
> >     gcc -v
> >     cat /etc/redhat-release
> >
> > If you see something like this:
> >
> > % uname -a
> > Linux prasad-fc5 2.6.15-1.2054_FC5smp #1 SMP Tue Mar 14 16:05:46 EST 2006 
> > i686 i686 i386 GNU/Linux
> > % gcc -v
> > Using built-in specs.
> > Target: i386-redhat-linux
> > Configured with: ../configure --prefix=/usr --mandir=/usr/share/man 
> > --infodir=/usr/share/info --enable-shared --enable-threads=posix 
> > --enable-checking=release --with-system-zlib --enable-__cxa_atexit 
> > --disable-libunwind-exceptions --enable-libgcj-multifile 
> > --enable-languages=c,c++,objc,obj-c++,java,fortran,ada 
> > --enable-java-awt=gtk --disable-dssi 
> > --with-java-home=/usr/lib/jvm/java-1.4.2-gcj-1.4.2.0/jre --with-cpu=generic 
> > --host=i386-redhat-linux
> > Thread model: posix
> > gcc version 4.1.1 20060525 (Red Hat 4.1.1-1)
> > % cat /etc/redhat-release
> > Fedora Core release 5 (Bordeaux)
> 
>    Yes, it looks pretty much the same.
> 
> >  bash
> >  CFLAGS='-g -O2 -fno-strict-aliasing' ./configure
> >  grep ^CFLAGS src/Makefile
> >  # You should see something like: CFLAGS = -g -O2 -fno-strict-aliasing
> >  make
> >
> > I believe that things should work fine for the compilation. If not,
> > please let us know what happens.
> 
>    I tried it, but nothing changed.

Okay, then you need to pursue Larry's suggestions about the bug in your
use of crypt().

> >  make check
> >
> > Well, there is no way that running this will do me any good. I do not
> > have and will NEVER run :pserver: on any machine of my own. I am not
> > a fan of it and never will be a fan of it.
> 
>    hmmn, now that's a question of personal choice, which dependes upon
> so many things,& so many factors, right? And trust me, I respect all
> of them equally. So, no offence !?! :)

Let me be more explicit. The sanity.sh tests will never require a user
to put a test version of cvs onto the net to see if it works without
some regression tests that exercise to the extent possible both the
client and pserver code paths in isolation.

The point is that your new 'passwd' sanity.sh test assumes that there
exists a repository that is running :pserver: that you can use as a part
of the test. This will never be true in our standard regression test
beds. So, if you don't provide stand-alone tests, this feature will not
go into the canonical source base.

I hope you understand. This one is a show stopper. There is no way that
I can verify that your test is working correctly without altering the
conditions of the testing that assumes no installed :pserver: instance
exists.

You should be able to borrow liberally from the current pserver-* tests
and maybe even the writeproxy tests for examples.

In the best of all possible worlds, you would provide the following
tests using something like

    dotest passwd-somelabel "$servercvs --allow-root=<root> pserver"

where you would be providing all of the server-side expected responses
to the client.

        * a new password where the 'servercvs' tells you it does not
          provide the 'passwd' 

        * the 'server' tells the client that it supports the CVSNT style
          Valid-requests and the client uses the CVSNT-style change to
          the password.

        * The same as above, but the errors that arise when the user
          uses the -e switch and that request is not available.

        * the 'server' tells the client that it supports the new CVS
          style '-e' encryption semantics (I don't seem to see which one
          you have added for that I only saw "passwd" added to the
          REQ_LINE lists. Recall that the client should be doing
          something like if (supported_request ("passwd-e")) {...} or
          such like.) and the client changes the password with the -e
          switch.

        * like the above, but the client does not use the -e switch.

another set of test might be written to exercise the servers responses
to valid and invalid client requests as well.

I do not have any time to try to write those tests for you myself.

> > different. It is possible that gcc may be sensative to other problems
> > such that writing the code this way:
> >
> >    case PASSWD:    /* change password */
> >        if (argc < 2)
> >            error (1, 0, "insufficient arguments");
> >        str = xstrdup (argv[1]);
> >        if (do_encrypt)
> >            pass = encrypt_pass (descramble (str));
> >        else
> >            pass = descramble (str);
> >        change_pass (usr, pass);
> >        free (str);
> >        break;
> 
>    I tried this also, but no change.
> 
> > Use' bool do_encrypt = false;' rather than 'short do_encrypt = 0;'
> > and 'do_encrypt = true;' rather than 'do_encrypt = 1;'
> >
> > I don't understand why mode is not of type enum op_mode given that you
> > take pains to define the enum right above it.
> >
> > in check_option(), be adivsed that 'c' should be an int rather than a
> > char (the getopt() function returns an int). You need to do this to
> > avoid problems on systems where char is signed.
> 
>    okay, I'll do the changes! :) :)
> 
> 
> Thank you so much for all the wonderful information!

You should also pay attention to Larry's suggestions regarding the
improper use of the crypt() function.

        -- Mark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.4 (FreeBSD)

iD8DBQFE6Uy/Cg7APGsDnFERAjCCAKCFB6KiZ3UGfcoj8eUlxT4WjQSUGwCg2pmM
7mF5I1zsb+0+TOyud0ET44E=
=1Gii
-----END PGP SIGNATURE-----