Hi Juergen and John,
I think it should be changed to malloc() space for the
qualifier as Juergen suggested.
The pseudo standard is hard to read as normal (IMHO:),
but it does indicate that we should call acl_free() when
we are done with it. (It does say to do this if it is releasable
memory - but how is the caller supposed to know whether it is
or isn't).
So we have no choice if we are supposed to call acl_free().
I will change it unless someone can convince me otherwise :)
Cheers,
--Tim
On Tue, Jun 19, 2001 at 04:52:36PM -0400, jtrostel@xxxxxxxxxx wrote:
> The standard states,
>
> "The acl_get_qualifier() function retrieves the qualifier of the tag for the
> ACL
> entry indicated by the argument entry_d into working storage and returns a
> pointer to that storage."
>
> "... Subsequent operations using the returned pointer shall operate on an
> independant copy of the qualifier in working storage.
>
> This function may cause memory to be allocated. The caller should
> free any releaseable memory, when the new qualifier is no longer required, by
> calling acl_free() with the void* as an argument."
>
> It looks like I designed the original function to just return a pointer.
> This,
> however, IS in working storage. The 'real' ACL lies beneath in/on the inode
> itself. The ACL we are working with here doesn't affect the 'real' ACL until
> it
> is written back to the inode/EA.
>
> The question is, what should the sentence about 'Subsequent operations ...'
> mean? If the user should be able to pull up numerous copies of this qualifier
> and work on them independantly without any cross influence, then I think
> Juergen is right ... again ;->
>
> It sure looks like a place rife for memory leaks though.... A malloc and no
> free.
>
> On 19-Jun-2001 Juergen Hasch wrote:
> >
> > There is a bug in acl.c/acl_get_qualifier. The Posix draft states
> > the object returned by acl_get_qualifier should be cleared using
> > acl_free. This crashes Samba because we only return a pointer from
> > inside an existing object.
> > The patch below fixes this, I somehow missed this before...
> >
> > --- acl.orig Fri Jun 8 22:53:48 2001
> > +++ acl.c Tue Jun 19 19:43:37 2001
> > @@ -996,6 +996,8 @@
> > void *
> > acl_get_qualifier (acl_entry_t entry_d)
> > {
> > + uid_t *retval;
> > +
> > if(entry_d == NULL){
> > setoserror(EINVAL);
> > return NULL;
> > }
> >
> > - return &entry_d->ae_id;
> > + retval = malloc(sizeof(uid_t));
> > + if (!retval)
> > + return NULL;
> > + *retval = entry_d->ae_id;
> > + return retval;
> > }
> >
> > int
>
> --
> John M. Trostel
> Linux OS Engineer
> Connex
> jtrostel@xxxxxxxxxx
|