[PATCH] Remove fstring from wb_acct_info
jra at samba.org
Wed Oct 31 22:19:12 UTC 2018
On Thu, Nov 01, 2018 at 11:15:12AM +1300, Andrew Bartlett via samba-technical wrote:
> On Wed, 2018-10-31 at 17:45 +0100, Samuel Cabrero via samba-technical
> > Hi,
> > the attached patch removes two fstrings from wb_acct_info struct. The
> > reason for this change is because the winbindd group enumeration
> > backend functions (ADS in particular) try to allocate an array of
> > wb_acct_info as long as the number of groups in the domain, which may
> > result in a huge chunk of memory for domains with a large number of
> > groups.
> > Branch:
> > https://gitlab.com/samuelcabrero/samba/commits/winbind_enum_grp_nomem
> > CI:
> > https://gitlab.com/samuelcabrero/samba/pipelines/34956873
> > Please review and push if you agree.
> I'm worried about memory lifetimes.
> Could you change the talloc_strdup() and normal assignments into
> talloc_steal() onto the head of the array at the memory context?
Actually I think the proposed code in the patch is good (I'm also reviewing
The talloc_strdup's are tallocing onto the (*info) struct,
which itself is talloc'ed off the mem_ctx. So they stay
around as long as the info struct does (they're owned by
> This would ensure the strings stay around as long as the structure
> pointing to them.
I think his patch already does that.
My only concern is that it changes the semantics so that
acct_desc can be NULL, whereas previously it was always
"" if not present.
Having said that I can't see any place were acct_desc
is used by the caller so I think it's good.
More information about the samba-technical