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. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
