Re: initial gencache implementation
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. 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. -- cheers, ++ |Rafal 'Mimir' Szczesniak [EMAIL PROTECTED] | |*BSD, GNU/Linux and Samba / |__/
Re: initial gencache implementation
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 [EMAIL PROTECTED] Manager, Authentication Subsystems, Samba Team [EMAIL PROTECTED] Student Network Administrator, Hawker College [EMAIL PROTECTED] http://samba.org http://build.samba.org http://hawkerc.net
initial gencache implementation
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
--
cheers,
++
|Rafal 'Mimir' Szczesniak [EMAIL PROTECTED] |
|*BSD, GNU/Linux and Samba /
|__/
Index: Makefile.in
===
RCS file: /cvsroot/samba/source/Makefile.in,v
retrieving revision 1.527
diff -u -r1.527 Makefile.in
--- Makefile.in 30 Aug 2002 12:46:54 - 1.527
+++ Makefile.in 5 Sep 2002 10:08:12 -
@@ -139,7 +139,7 @@
lib/md5.o lib/hmacmd5.o lib/iconv.o lib/smbpasswd.o \
nsswitch/wb_client.o nsswitch/wb_common.o \
lib/pam_errors.o intl/lang_tdb.o lib/account_pol.o \
- lib/adt_tree.o lib/popt_common.o $(TDB_OBJ)
+ lib/adt_tree.o lib/popt_common.o lib/gencache.o $(TDB_OBJ)
LIB_SMBD_OBJ = lib/system_smbd.o lib/util_smbd.o
@@ -242,7 +242,8 @@
AUTH_OBJ = auth/auth.o auth/auth_sam.o auth/auth_server.o auth/auth_domain.o \
auth/auth_rhosts.o auth/auth_unix.o auth/auth_util.o auth/auth_winbind.o \
- auth/auth_builtin.o auth/auth_compat.o $(PLAINTEXT_AUTH_OBJ) $(UNIGRP_OBJ)
+ auth/auth_builtin.o auth/auth_compat.o \
+ $(PLAINTEXT_AUTH_OBJ) $(UNIGRP_OBJ)
MANGLE_OBJ = smbd/mangle.o smbd/mangle_hash.o smbd/mangle_map.o smbd/mangle_hash2.o
@@ -381,7 +382,8 @@
NET_OBJ1 = utils/net.o utils/net_ads.o utils/net_ads_cldap.o utils/net_help.o \
utils/net_rap.o utils/net_rpc.o utils/net_rpc_samsync.o \
- utils/net_rpc_join.o utils/net_time.o utils/net_lookup.o
+ utils/net_rpc_join.o utils/net_time.o utils/net_lookup.o \
+ utils/net_cache.o
NET_OBJ = $(NET_OBJ1) $(SECRETS_OBJ) $(LIBSMB_OBJ) \
$(RPC_PARSE_OBJ) $(PASSDB_GET_SET_OBJ) \
/*
Unix SMB/CIFS implementation.
Generic, persistent and shared between processes cache mechanism for use
by various parts of the Samba code
Copyright (C) Rafal Szczesniak2002
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program; if not, write to the Free Software
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
#include includes.h
#undef DBGC_CLASS
#define DBGC_CLASS DBGC_TDB
#define TIMEOUT_LEN 12
#define CACHE_DATA_FMT %12d; %s
static TDB_CONTEXT *cache;
/**
* @file gencache.c
* @brief Generic, persistent and shared between processes cache mechanism
*for use by various parts of the Samba code
*
**/
/**
* Cache initialisation function. Opens cache tdb file or creates
* it if does not exist.
*
* @return true on successful initialisation of the cache or
* false on failure
**/
BOOL gencache_init(void)
{
char* cache_fname;
/* skip file open if it's already opened */
if (cache) return True;
asprintf(cache_fname, %s/%s, lp_lockdir(), gencache.tdb);
DEBUG(5, (Opening cache file at %s\n, cache_fname));
cache = tdb_open_log(cache_fname, 0, TDB_DEFAULT,
O_RDWR|O_CREAT, 0644);
if (!cache) {
DEBUG(0, (Attempt to open the cache file has failed.\n));
return False;
}
return True;
}
/**
* Cache shutdown function. Closes opened cache tdb file.
*
* @return true on successful closing the cache or
* false on failure during cache shutdown
**/
BOOL gencache_shutdown(void)
{
/* tdb_close routine returns 0 on successful close */
if (!cache) return False;
DEBUG(5, (Closing cache file\n));
return tdb_close(cache) ? False : True;
}
/**
* Add one entry to the cache file.
* (it part of tridge's proposed API)
*
* @param key string that represents a key of this entry
* @param value text representation value being cached
* @param timeout time when the value is expired
*
* @return true when entry is successfuly stored or
* false on the attempt's failure
**/
BOOL gencache_add(const char *keystr, const char *value, time_t timeout)
{
int ret;
TDB_DATA keybuf, databuf;
char* valstr = NULL;
if (!gencache_init()) return False;
asprintf(valstr, CACHE_DATA_FMT, (int)timeout, value);
keybuf.dptr = strdup(keystr);
keybuf.dsize = strlen(keystr);
databuf.dptr = strdup(valstr);
databuf.dsize = strlen(valstr);
DEBUG(10, (Adding cache entry with key = %s; value = %s and timeout
gencache implementation
I forgot to send patch with net_cache's entrypoint.
--
cheers,
++
|Rafal 'Mimir' Szczesniak [EMAIL PROTECTED] |
|*BSD, GNU/Linux and Samba /
|__/
Index: utils/net.c
===
RCS file: /cvsroot/samba/source/utils/net.c,v
retrieving revision 1.56
diff -u -r1.56 net.c
--- utils/net.c 21 Aug 2002 19:39:38 - 1.56
+++ utils/net.c 5 Sep 2002 14:14:28 -
@@ -352,6 +352,7 @@
{TIME, net_time},
{LOOKUP, net_lookup},
{JOIN, net_join},
+ {CACHE, net_cache},
{HELP, net_help},
{NULL, NULL}
Index: utils/net_help.c
===
RCS file: /cvsroot/samba/source/utils/net_help.c,v
retrieving revision 1.7
diff -u -r1.7 net_help.c
--- utils/net_help.c25 Jun 2002 02:29:09 - 1.7
+++ utils/net_help.c5 Sep 2002 14:14:29 -
@@ -135,6 +135,7 @@
net user\t\tto manage users\n\
net group\t\tto manage groups\n\
net join\t\tto join a domain\n\
+ net cache\t\tto operate on cache tdb file\n\
\n\
net ads command\tto run ADS commands\n\
net rap command\tto run RAP (pre-RPC) commands\n\
Re: initial gencache implementation
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. (-: You assume that any cached data will be in null terminated string format which is not always the case. This is just my personal opinion but I would prefer for gencache_set to 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. Some other minor things: - memleak of cache_fname in gencache_init - memleak of keybuf.dptr in gencache_set 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. Tim.
Re: initial gencache implementation
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. (-: You assume that any cached data will be in null terminated string format which is not always the case. I understand this is a design property - it's up to the caller to mess with structs etc. This keeps the cache simple, and allows for human inspection with 'net cache' etc. This is just my personal opinion but I would prefer for gencache_set to 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. Agreed. Andrew Bartlett
Re: initial gencache implementation
On Fri, Sep 06, 2002 at 02:10:23AM +, [EMAIL PROTECTED] wrote: You assume that any cached data will be in null terminated string format which is not always the case. I understand this is a design property - it's up to the caller to mess with structs etc. This keeps the cache simple, and allows for human inspection with 'net cache' etc. OK - fair enough, I missed that bit. Tim.
