NULL check after malloc missing , memory leak and other

[andreas moroder]

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