NULL check after malloc missing , memory leak and other
andreas moroder
claudiamoroder at st-ulrich.suedtirol.net
Sat Aug 4 15:09:44 GMT 2001
Hello,
I think I have found a missing check after a alloc and a small memory leak in
2.2.1a
in lib/messages.c there are the following lines
void message_register(int msg_type,
void (*fn)(int msg_type, pid_t pid, void *buf, size_t len))
{
struct dispatch_fns *dfn;
dfn = (struct dispatch_fns *)malloc(sizeof(*dfn));
ZERO_STRUCTP(dfn);
....
dfn is used without a check if memory is allocated.
#################################
in lib/util_file.c there are the following lines
char *fgets_slash(char *s2,int maxlen,FILE *f)
{
char *s=s2;
int len = 0;
int c;
BOOL start_of_line = True;
if (feof(f))
return(NULL);
if (!s2)
{
maxlen = MIN(maxlen,8);
s = (char *)Realloc(s,maxlen);
}
if (!s || maxlen < 2) return(NULL);
if the parameter s2 is null and maxlen=1 ( I don't know if this can happen,
but if it can't there would probably be no check ) memory is allocated and
assigned to s but in the next line the function returns leaving around the
memory allocated
#################################
There is a function Realloc defined and used 105 times, but why is realloc
also used 24 times ? Are they different, and if, why are they used both ?
#################################
And now a performance questions
in lib/util.c there are the following lines
SMB_OFF_T transfer_file(int infd,int outfd,SMB_OFF_T n,char *header,int
headlen,int align)
{
static char *buf=NULL;
static int size=0;
.......
while (!buf && size>0) {
buf = (char *)Realloc(buf,size+8);
if (!buf) size /= 2;
}
why is Realloc used here, instead of malloc, Realloc tests buf that is
already always NULL and the size+8 that is always >=8. It should be faster to
use malloc.
#################################
And now a few questions about the whole talloc system
in talloc.c i found the lines
/* allocate a bit of memory from the specified pool */
void *talloc(TALLOC_CTX *t, size_t size)
{
void *p;
struct talloc_chunk *tc;
if (size == 0) return NULL;
p = malloc(size);
if (!p) return p;
tc = malloc(sizeof(*tc));
if (!tc) {
free(p);
return NULL;
}
tc->ptr = p;
tc->size = size;
tc->next = t->list;
t->list = tc;
t->total_alloc_size += size;
return p;
}
What about to:
- allocate one memory block with the size size(TALLOC_CTX) alligned to 8 or
what is needed + size and return the pointer + size(TALLOC_CTX) alligned
This way there is only one malloc so the code is faster
- work with a double linked list
Because of the possibility to realloc you need a doubly linked list, so you
can reallocate new memory. If you would not use double linked list, there is
now way to exchange the old entry with the new one.
realloc would also be very much faster, because there is no need to search
the entry in the list, you always now it because you now it is pointer -
size(TALLOC_CTX) alligned.
If did only read a little bit of the code, so I don't know if there are
already functions to manage double linked lists.
Bye
Andreas Moroder
More information about the samba-technical
mailing list