initial gencache implementation

Andrew Bartlett abartlet at samba.org
Fri Sep 6 13:42:00 GMT 2002


Rafal Szczesniak wrote:
> 
> On Fri, Sep 06, 2002 at 10:17:19AM +1000, Tim Potter wrote:
> > On Thu, Sep 05, 2002 at 12:15:36PM +0200, Rafal Szczesniak wrote:
> >
> > > This is first implementation of caching mechanism. It includes
> > > both lib/gencache.c code and utils/net_cache.c as command-line
> > > control/testing tool.
> > >
> > > comments are welcome
> >
> > Rafal, that looks pretty good.  Since you ask, I do have a few comments.
> > (-:
> 
> I'm glad to hear that :)
> 
> > You assume that any cached data will be in null terminated string format
> > which is not always the case.
> 
> I assume that on gencache base we can implement any higher-level
> caching function like namecache. Then, it's up to such implementation
> how to 'pack' the structures into string form. Null terminated string
> format is much easier to watch with 'net cache list' command. Thus
> we have comfortable and easy mean to watch what's in the cache file.
> 
> > This is just my personal opinion but I would prefer for gencache_set to
> > crash
> 
> To avoid mistake, I should ask what exactly do you mean by 'crash' ?
> 
> > if you pass it a NULL pointer for the key or value parameter.
> > Returning false in this case only hides the error until further along
> > in the execution path by which time it will be more difficult to track
> > down the original error.
> 
> Good point. Just explain me this 'crash' thing.

SMB_ASSERT or smb_panic().  Causes the program to exit, and calls the
panic action.  Good for debugging - and people complain fast if it's
broken in the feild.

> > Some other minor things:
> >
> >       - memleak of cache_fname in gencache_init
> >       - memleak of keybuf.dptr in gencache_set
> 
> Thank you. The latter was already fixed - I just forgot to send fixed
> version :)
> 
> > I don't think you need to strdup the key before passing it to tdb_fetch
> > in gencache_set.  You can just use the passed in parameter.
> 
> I thought about that but unfortunatelly tdb_store doesn't have const
> args, so compiler complains about passing pointers to non-const args.
> I think tdb_store should have const-ed args (since it copies them anyway),
> but it's quite other topic.

I would like to see a patch for this at some stage - it frustrates me
too...

Andrew Bartlett
-- 
Andrew Bartlett                                 abartlet at pcug.org.au
Manager, Authentication Subsystems, Samba Team  abartlet at samba.org
Student Network Administrator, Hawker College   abartlet at hawkerc.net
http://samba.org     http://build.samba.org     http://hawkerc.net



More information about the samba-technical mailing list