On Wed, Jun 06, 2012 at 09:40:44AM +0200, Sumit Bose wrote:
On Wed, Jun 06, 2012 at 08:03:28AM +0200, Jakub Hrozek wrote:
> On Tue, Jun 05, 2012 at 06:34:20PM +0200, Sumit Bose wrote:
> > On Mon, Jun 04, 2012 at 02:49:48PM +0200, Sumit Bose wrote:
> > > On Mon, Jun 04, 2012 at 02:15:37PM +0200, Jakub Hrozek wrote:
> > > > On Fri, Jun 01, 2012 at 01:06:11PM +0200, Sumit Bose wrote:
> > > > > On Thu, May 31, 2012 at 09:09:53AM +0200, Jakub Hrozek wrote:
> > > > > > On Thu, May 24, 2012 at 03:04:36PM +0200, Sumit Bose
wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > this patch allows us besides other conversions to
convert the dom_sid
> > > > > > > structure used by samba to strings and back. This
structure is used by
> > > > > > > various samba libraries, but there are no public
inferfaces for the
> > > > > > > conversion. I've seen Simo adding code to the IPA
kdb plugin doing these
> > > > > > > conversions and I need them for the PAC responder as
well. So I thought
> > > > > > > it might be useful to put it in a library.
> > > > > > >
> > > > > > > bye,
> > > > > > > Sumit
> > > > > >
> > > > > > Hi, the patch no longer applies to configure.ac, can you
rebase?
> > > > >
> > > > > sure, new version attached.
> > > > >
> > > > > bye,
> > > > > Sumit
> > > >
> > > > The code looks good to me and the unit tests pass.
> > > >
> > > > But the patch adds a new build-time dependency that should be added
into
> > > > contrib/sssd.spec.in. I also wonder if there should be a
configure-time
> > >
> > > ah, I have this in the PAC responder patch. I will extract it there and
> > > add it here.
> > >
> > > > switch to compile this feature in or out, mainly because nothing is
> > > > using the feature at the moment?
> > >
> > > When starting the patch I was thinking the same, but then I realized
> > > that this would change the API/ABI of the library. So the samba header
> > > files are now required at compile time, but not at run time.
> > >
> >
> > After some discussion on irc it became clear that we what to avoid any
> > kind of dependencies here, because the needed header files are often not
> > packaged. The new version of the patch copies the definition of the
> > struct to reach this.
> >
> > bye,
> > Sumit
>
> I'm not sure the new functions should use talloc_free() directly, I
> think they should use ctx->free_func, there's no guarantee that
> alloc_func will be talloc.
Thank you for catching this, you are completely right, new version
attached.
bye,
Sumit
Ack