[Pkg-samba-maint] Debian local patches and Samba

[Michael Tokarev]

08.05.2022 23:11, Andrew Bartlett wrote:
> Michael,
..> In the past it has been the strong practice in Samba packaging on
> Debian not to include local patches, except where those have already
> been accepted upstream (eg backports from master).
..

FWIW, there is one patch in debian which I haven't posted anywhere,
it is use-bzero-instead-of-memset_s.diff .  In lib/replace/'s
ZERO_STRUCT() samba uses memset_s which it provides within libreplace.
BSD has memset_s iirc, so it is a no-op on there, it does not need to
be provided (bzero also come from BSD iirc).  But on linux there's no
memset_s, but bzero() is available. By using bzero() instead of memset_s
in this place we can avoid linking libreplace into certain binaries
which does not actually use any of their functions, and this way we
can make individual binary packages with less inter-dependencies.

I don't know if this is a material for upstream samba. It helped me
to break circular dependency between libwbclient0 and samba-libs.

It probably helps to have some CI testing for replacing memset_s()
with bzero(). Dunno how important it is, though.

> The rationale has been that this avoids divergence, forward-porting and
> allows upstream to express and experienced judgement regarding the
> patches.  It also ensures that, at least in combination with git
> master, the patch passes CI (avoiding unwanted side-effects).
> 
> I think this has served us well, and I would continue to encourage the
> submission of patches upstream via GitLab per
> https://wiki.samba.org/index.php/Contribute
> 
> If you have any difficulties with this process please discuss on samba-
> technical, we remain glad to help ensure Samba is as good as possible
> for Debian.

It is not about debian, not at all. All I do is not debian-specific.

It is the general code quality and, so to say, architecture.
Just too many places in samba are badly thought and later done
without much thinking too.  For example the bugfix which I mentioned
when asked about msg.sock/ directory - in there, someone fixed a
bug - a null-ptr deref when some *alloc() returned null, but they
did not fix exactly the same bug in the *previous line*!! C'mon,
this is just a bad way to do things, and no one from the reviewers
noticed that too.. There are many other places in nearby parts of
this very file too which needs exactly the same fixes - just a
trivial search gives quite some results. But a nearby line? -
that's.. something :)

I understand it is a very large codebase and it is often difficult
to make changes without being afraid to break something else. So
instead of fixing some bigger problem it sometimes feels easier
to work around it in a place where it hurts. The result becomes..
more and more wrong.  In Russian we have a word for this, it is
"Тришкин кафтан" - a caftan which consists only from patches in
the end, ready to fall into pieces. CI does not help with the
general design.

In this context I'm really proud of the samba team for fixing
the big symlink issue by rewriting the vfs layer.  It was the
very right thing to do, despite its complexity.

Thanks,

/mjt