httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@covalent.net>
Subject Re: [REWRITE] htpasswd+htdbm+dbmmange
Date Tue, 06 Nov 2001 21:47:35 GMT
From: "Mladen Turk" <mturk@mappingsoft.com>
Sent: Tuesday, November 06, 2001 1:17 PM


> > > 1. htpasswd.c
> > > a) uses the same command line options as old one
> > > b) the default is MD5 on all platforms
> > > c) has tree new options: list, delete and verify(check)
> >
> > I'd commented on this once before, it was too major of an
> > overhaul to follow
> > in cvs versioning.  I didn't (don't) see the need for such an
> > extensive rewrite.
> >
> 
> Well, after all, It's a rewrite. I don't think that the new version of
> software must look like the old one except from the user perspective. It's
> some times easier to build the things from ground-up then messing with
> someone other code. APR-izing the old htpasswd isn't such a big deal now
> that we have apr_mktemp, but as you noticed I've tried to introduce a new
> concept to user/group management.

I didn't see the purpose of a rewrite.  Then again, I didn't catch the fact
that you introduced group management, ergo refactoring appeared pointless ;)

> > > 2. htdbm.c
> >
> >   1. changes to stdout?  I don't get it.  If we must, let's use
 > >      file name '-' as many unix utilities do.
> 
> Same syntax from htpasswd...

Ahhh... so simply for 'appearences'.  I'd suggest it's of negligable value
in this context (we are managing files, after all ;)

> >
> >   2. -r makes more sense for remove (-d being taken already.)
> 
> Just change the the switch...

Done after my next commit.

> >   3. the code is rather, well, dirty, from the perspective that
> > we overload this
> >      htdbm.c with all of the trappings of an extension library.
> > I'm absolutly -1
> >      on this approach.  If you want to create an ap_htdbm.c
> > library, and link it
> >      to htdbm.c (using that api) then fine.
> 
> That was my thoughts in general. If you look at the code the htpasswd, htdbm
> and htxml utilities main() functions differentiates very little from each
> other (mostly command param options parsing).
> All that can easily go to a library as in fact does for my GUI user/group
> management tool.

Ok, then we need a wholly different approach.  I'm refactoring your code in
the following directions;

  1. reusing code we already have (p/w packing (to64) and getword) rather than
     quoting it again.

  2. splitting the htdbm_t from the userdatum_t from the usertable_t, so that
     all have the same datum (name/groups/pw/comment) even if parts are 
     unsupported, and the htdbm_t will become transparent, while usertable may
     be opaque.

  3. always recognize -d, spit out that it's unsupported on a given platform
     (and still, as you did, not present it as an option if its not supported.)

> >   4. you forgot such bits as initializing apr, the pool, etc.
> > I'm already experimenting,
> >      and happy to clean those up (such as using apr_getopt, etc,
> > to shrink the code, using
> >      a single char*username rather than the int gave_username
> > approach, etc.)
> 
> Take a second look :-)
> apu_xxxx_init calls the apr_initialize (APU_XXXX_STANDALONE flag).

Ahhhh... yes, but those are mainline, so I'm refactoring them into main(),
even as a library these would never be called except by the parent app.

> You cannot use the apr_getopt and be backward compatible with the htpasswd
> for example, because the old one can have "-bc" or "-b -c" as valid params
> and using apr_getopt the valid ones are only "-b -c"...

Then we look at extending apr_getopt.  We eat our own dogfood :)  That will
be in a later commit, it's relatively low priority.

> >   5. groups and comments are an essential part of dbm management
> > (and a key attraction)
> >      for many users.  Again, I'm happy to round that out.
> 
> htdbm can easily manage comments (the -t option).
> groups can be managed using plain encryption and using comma separated group
> names instead password, but I agree that this can be done more user
> friendly.

Ack.  And that's the 'phase 2' of the refactor.  But I'm growing to like the
way you started to modularize, I'll just take it further to where we can split
out a library to have a single htusers_t supporting htdbm_t, htfile_t, ht_xml_t
and whereever you would like to take it.

I don't see a reason to make this 'library' until 2.1, but it should be trivial
with some small modifications to your code.  ITMT, I've stripped the declares,
and we can put them back once this is a seperate source file for inclusion in
any dbm manager (and mod_auth_dbm itself.)

Bill



Mime
View raw message