NULL check after malloc missing , memory leak and other

[Simo Sorce]

thank you,
I've fixed 1,2,4 and I'm investigating about why there are both reallocs and Reallocs
About talloc, talloc mean trivial alloc, single mallocs are not performance bottlenecks, while doubly-linked list are. Managing them cost some time.
Adding or substracitng many times to find a pointer every time it is needed
will certainly degrade performances more than adding a malloc IMHO.

Simo.

On Sat, Aug 04, 2001 at 05:09:44PM +0200, andreas moroder wrote:
> 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
> 
> 
> 
> 
> 
> 
> 
> 
> 

-- 
Simo Sorce       idra at samba.org
-------------------------------
Samba Team http://www.samba.org