[PATCH] Cleanup - remove unused functionality. -- OpenChange break

Alexander Bokovoy ab at samba.org
Tue Apr 18 18:42:06 UTC 2017


On ti, 18 huhti 2017, Jeremy Allison wrote:
> On Tue, Apr 18, 2017 at 09:07:26PM +0300, Alexander Bokovoy wrote:
> > On ti, 18 huhti 2017, Jeremy Allison wrote:
> > > On Tue, Apr 18, 2017 at 08:46:48PM +0300, Alexander Bokovoy wrote:
> > > > On ti, 18 huhti 2017, Jeremy Allison via samba-technical wrote:
> > > > > More removal of talloc_autofree_context()
> > > > > from completely unused code.
> > > > > 
> > > > > I'm planning to remove this entirely from
> > > > > our codebase so we can make a start on adding
> > > > > threaded code into Samba.
> > > > > 
> > > > > Please review and push if happy !
> > > > I'm slightly worried about removal of the lpcfg_register_defaults_hook()
> > > > and friends. Sure, Openchange seems to be dead but people still use
> > > > Zentyal's fork which was updated a year ago.
> > > 
> > > It's a horrible external abuse of our code, which
> > > prevents us from modernizing our internals.
> > > 
> > > I want rid of these monkeys on our back.
> > All I want to see is a louder announcement that shatter is coming.
> > Without being sound how could they know it is a scheduled drop?
> > Announcing the removal under "unused functionality" cleanup is living us
> > without any towel. Have you put these plans somewhere else half a year
> > ago, perhaps?
> 
> This is for 4.7+. It's not like I'm removing from
> current releases.
> 
> I can add a note to the WHATSNEW if you like ?
Yes, please add the note and then my RB+.
 
> The problem is this is a one-project-private
> hack that somehow has become embedded in Samba
> code and is now a barrier to fixing real bugs
> and modernizing our code.
> 
> If it's essential to Zentyal's fork they
> need to carry it as a private patch, that's
> all I'm saying. That's what a fork means
> after all. This interface isn't promised
> as a public ABI.
> 
> Everyone else does the same for custom changes they
> need.
I do agree with this approach. As I said, I wanted this to be visible on
the list rather than burried in the code. Process-related policies not
necessarily good when only enforced through the code.
 
> > We have talloc_autofree_context() used by SSSD as well. Are you planning
> > to kill talloc_autofree_context() too? Not that SSSD needs it
> > desperately but it is used there.
> 
> Not at all ! talloc_autofree_context() is part of
> our external talloc ABI and will never go away.
> 
> I just don't want it used inside Samba code,
> that's all.
> 
> The reason is that it installs a global atexit()
> handler and when used with talloc_enable_null_tracking()
> (which Samba code currently does, I want rid
> of that too :-) it makes it *impossible* for
> talloc code to be used safely by threads.
> 
> We have to fix this for Samba to move us
> forward.
Yep. Please note that some issues cannot be solved without glibc be
fixed. MIT Kerberos, for example, loads plugins and makes sure they are
unloaded when not used anymore. This freaks out glibc code due to
undefined behavior in ELF spec. It was fixed recently (December 2016) in
Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=1398370

-- 
/ Alexander Bokovoy



More information about the samba-technical mailing list