[PATCHES v2] Fix FreeBSD developer build

Andrew Bartlett abartlet at samba.org
Wed Nov 22 01:18:16 UTC 2017

On Wed, 2017-11-22 at 13:15 +1300, Andrew Bartlett via samba-technical
> On Tue, 2017-11-21 at 21:25 +0200, Uri Simchoni via samba-technical
> wrote:
> > Hi,
> > 
> > Attached version 3 - same as v2, but with added 3 patches at the end,
> > which allow FreeBSD developer build to run without the ADDITIONAL_CFLAGS
> > workaround, and without breaking Linux...
> > 
> > Some responses below.
> > 
> > Thanks,
> > Uri.
> > 
> > On 11/21/2017 12:58 AM, Timur I. Bakeyev via samba-technical wrote:
> > > Hi, Uri!
> > > 
> > > That's quite a jumbo patch, possibly splitting it into smaller chunks could
> > > be easier to review :)
> > > 
> > 
> > That's a jumbo patch-*set*. Each patch is pretty small, concerns one
> > issue, and most patches modify a single file. I believe that's more or
> > less the Samba practice. Granted, it's easier for people to review a
> > small patch set, but that would mean I'd get into a cycle of several
> > weeks, waiting for the previous subset to land before submitting the
> > next one - I don't have the patience for that so let's keep our fingers
> > crossed.
> I've taken a go at it.  I'll review what I can of this, so as to cut
> down the set into the changes I can simply approve. 
> In particular, I've dropped the pragma changes.  #pragma caused chaos
> last time we used it, so I'm skipping those for now.

One change that I've reviewed, but I'm not very happy with is this one:

 sysacls: make SMB_ACL_PERMSET_T opaque
    sys_acl_get_permset() returns a pointer to a uint32_t. Prior
    to this change, SMB_ACL_PERMSET_T has been defined as a mode_t
    pointer, where mode_t may or may not be uint32_t (on FreeBSD
    it's uint16_t, which could screw up big-endian FreeBSD systems).
    Since SMB_ACL_PERMSET_T is being used as an opaque
    data type to the ouside world, this patch changes it
    to a void*, and in the implementation casts the pointer
    to a uint32_t pointer.
    Signed-off-by: Uri Simchoni <uri at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

Could you see if you could re-do it to instead change the interface
where the mode_t matters, and fix it as a uint32_t in the type.  void*
is to be avoided if possible. 

Can you also explain more how the change in the type hurts things?

If it can't be done, then my review stands, but can you take a look

In the meantime I've pushed the rest of this partial set to autobuild,
hopefully that makes review of the remainder easier.


Andrew Bartlett

Andrew Bartlett
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   

More information about the samba-technical mailing list