[SSSD] [PATCH] Rename SAFEALIGN macros

Jakub Hrozek jhrozek at redhat.com
Fri Apr 26 08:21:25 UTC 2013


On Thu, Apr 25, 2013 at 07:17:20PM -0400, Simo Sorce wrote:
> On Fri, 2013-04-26 at 00:51 +0200, Jakub Hrozek wrote:
> > On Thu, Apr 25, 2013 at 03:41:28PM +0200, Michal Židek wrote:
> > > https://fedorahosted.org/sssd/ticket/1772
> > > 
> > > Changes done:
> > > - definitions of safealign macros have been removed from
> > > src/sss_client/sss_cli.h and src/util/util.h and put into new header
> > > (the old headers include this new header). This change was done to
> > > avoid code duplication.
> > > 
> > > - Macros that copy bytes from a variable to byte buffer have been
> > > renamed from SAFEALIGN_SET_<type> to SAFEALIGN_VAR2BUF_<type>.
> > > 
> > > - Macros that copy bytes from a byte buffer to a variable have been
> > > renamed from SAFEALIGN_COPY_<type> to SAFEALIGN_BUF2VAR_<type>
> > > 
> > > - Aliases have been added to allow the old names to be used in the
> > > code (we can remove the aliases when the old names are replaced with
> > > the new ones on all places in the code, but for now, it is good to
> > > allow both alternatives, so that this patch can be smaller)
> > > 
> > > Thanks
> > > Michal
> > 
> > You need to add the new header to dist_noinst_HEADERS in Makefile.am
> > otherwise you break make distcheck.
> > 
> > Also, I was thinking about how we create the headers for consumption in
> > both server and the client -- would it be a good idea to differentiate
> > them on the filesystem level so that it's immediatelly clear that this
> > header is special?
> 
> I agree this is important.
> The header was put in sss_client to make crystal clear this header need
> to stay slim and can't grow dependencies and cruft as the client library
> needs to stay minimal.
> 

I think the code is minimal, it's just macros.

> In moving this header we need a way to maintain this notion.
> 
> > I see two options:
> >     a) create a new subdirectory, something like src/shared (better name
> >     welcome) that would contain the code shared between server and client
> >     b) use some kind of prefix in the filename such as cs_header.h
> 
> We already have another header in util/ in the same situation.
> At the time we opted for adding a prominent comment in the header file
> (option c).
> We can do the same here, or we can decide to go with a)

That's what Michal did, the comment was there.

The only thing I was afraid of was that someone would navigate with ctags
into middle of the file and wouldn't see the comment on top. Then again,
this is something a code review should catch, too.



More information about the sssd-devel mailing list