[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
wrote:
> 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
again?

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

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba







More information about the samba-technical mailing list