[splint-discuss] About onlytrans

[Tommy Pettersson]

On Thu, Dec 13, 2007 at 11:22:28AM +0800, Ma Li Yu wrote:
> Currently I faced below issue:
> 
> pdis.c(115,3): Only storage assigned to unqualified static:
> 
>                   Key_ptr = (KEY_BUFFERING*)malloc(sizeof(KEY_BUFFERING))
> 
>   The only reference to this storage is transferred to another reference
> (e.g., by returning it) that does not have the only annotation. This may lead
> to a memory leak, since the new reference is not necessarily released. (Use
> -onlytrans to inhibit warning)
> 
> This is definition of Key_ptr. It was defined outside of function
> Store_Key:
> 
> Static KEY_BUFFERING * Key_ptr;
> 
> It tries to tell me that it could lead to a memory leak. But I did not
> understand it. The strange thing is that splint would not complain any
> more after I changed to below code:
> 
> 
> KEY_BUFFERING *  temp_ptr; //Define inside function Store_Key
> 
> ....
> 
> temp_ptr = (KEY_BUFFERING *)malloc(sizeof(KEY_BUFFERING));
> Key_ptr = temp_ptr;
> 
> 
> But I think it is not optimized. Above code is simpler and take less
> code size. What did u think? 

I think the optimizer in the compiler will completely remove the
redundant temp_ptr, but that's not the point here.

If you don't put splint annotations before the pointers to tell
splint how you intend to manage the memory, splint will assume
some defaults, and check the code based on these assumptions.
There are also limitations to how much splint can check global
variables; splint checks that sanity is preserved between the
entry and the exit of a function, but it knows little or nothing
about the run-time call graph, and thus the state of a global
variable at function entry.

Anyway, the automatic variable temp_ptr will default to Only,
which makes splint happily transfer the ownership of the
allocated memory from malloc to temp_ptr. I don't remember what
static globals default to, but it's probably some kind of shared
type, i.e., when you assign Key_ptr=temp_ptr, temp_ptr is
holding on to the ownership while Key_ptr just becomes an alias.
This means 1) you must free(temp_ptr) before temp_ptr goes out
of scope, or pass it on to some other Only pointer 2) you can
not free(Key_ptr).

Whit the temp_ptr "trick" you'll get warnings from splint at the
function exit that temp_ptr is not freed, and that Key_ptr may
reference Null storage. So it doesn't really solve anything.

In the original case, the Only pointer returned from malloc is
assigned directly to the unannotated Key_ptr, which has no
responsibility to free the memory, so the ownership of the
memory is lost -- there are no longer any pointer you can use to
free the memory (the way splint sees it, because you have not
told splint that Key_ptr is an Only pointer). That is why splint
reports a memory leak.

If you annotate Key_ptr with /*@only@*/, things will work more
like you expect, but then you will instead get a warning about
"Only storage Key_ptr not released before assignment, a memory
leak has been detected". This is because, as you remember,
splint does not know about the state of global variables at
function entry, so it assumes Key_ptr already holds a pointer to
some memory, which will be lost and cause a memory leak when you
overwrite it with the new pointer from malloc. There is also
still the warning about Key_ptr may become Null.

Try this:

  static /*@only@*/ /*@null@*/ buf_t * Key_ptr;
  
  ...
  
  if (Key_ptr)
    free( Key_ptr );
  Key_ptr = malloc( BUF_SIZE );
  
Now splint is confident that no memory is leaked.


There are other memory management models with other annotations
than Only in splint, which may be more appropriate in your case,
and there are ways to annotate functions to explains to splint
that parameters are unassigned at function entry and must be
assigned before function exit. Such annotations will catch
errors where functions are called in inappropriate ways, like:

  ptr = malloc( size );
  do_allocate( &ptr );

An if (p) free(p) trick in do_allocate would probably not be so
good. It would be preferred that splint warned about do_allocate
being called in a place where ptr was already assigned.
Unfortunately I think there are some limitations on global
variables with those kind of annotations.


You should first try to learn the different ways splint thinks
memory can be managed safely. This will make the warning
messages much more understandable. They you must realize splint
is limited in the way it can model memory management, since it
can only do static checks. There are many common ways of
managing memory that splint can not fully check. There is often
a choice between using memory management that splint understands
to get full checking, or use a more convenient model that splint
can only partly check is correct.


-- 
Tommy Pettersson <ptp at lysator.liu.se>