[SSSD] Coding standard question (was ELAPI Adding file... )

Dmitri Pal dpal at redhat.com
Fri Sep 4 16:38:37 UTC 2009


Stephen Gallagher wrote:
> On 09/02/2009 08:27 PM, Dmitri Pal wrote:
> > Hi,
>
> > This patch is next significant chunk of code.
> > See patch comments for the main description.
> > It requires previous patch that has been reviewed but was not pushed
> yet.
> > It addresses some of the suggestions risen during the review.
> > I also added couple tickets: to eventually remove all the "Fixme"
> > comments and to refactor places where I allocated context structs and
> > initialize them.
>
> > Checked with make check, valgrind and did in tree and parallel builds.
> > All works, no leaks.
>
> > Sorry that it is too big.
> > I need to move on and there are many things that are still missing.
> > I will start working on  the next set of the functionality tomorrow
> > mainly adding async processing. That would definitely affect some of the
> > arguments passed all along so expect changes to many modules.
>
>
>
> > ------------------------------------------------------------------------
>
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel at lists.fedorahosted.org
> > https://fedorahosted.org/mailman/listinfo/sssd-devel
>
> Nack.
>
> Please do not use a #define when you really mean "typedef"
>
> you probably wanted
> typedef struct elapi_data_out {
>     char *buffer;
>     uint32_t size;
>     uint32_t length;
>     uint32_t written;
> } SEDO;
>
> But I still don't much care for SEDO. But this is an internal-only
> header so I'm not going to complain too loudly. That said, we generally
> frown upon using typedef anywhere but function pointers, since they
> obfuscate the actual type you're using. I'd much prefer if you used
> "struct elapi_data_out" written out fully.
>
> This is the only problem I see in a high-level overview of this code.
> Jakub was going to do a more in-depth review, so I'll wait on his
> comments as well.
>
Hm, I was testing you :-)
I did not do the typedef exactly because the of the policy you mentioned.
Simo mentioned during the coding style discussions that he would prefer
defines to typedes since the debugger in such cases works correctly
and shows the contents of the structure while the typedef
obfuscates things.
The spelled out struct made the lines pretty long so I tried
to shorten it. And then it occurred to me that I might just
use a define.
The only thing that is not correct in your assessment
is that it is completely internal. It is internal and low level but
will be a part of the helper suit that the developers of the
externally loadable ELAPI providers would have to use
so it will be exposed.
Keeping this in mind I want an answer to a more general
question: do we think such #defines are acceptable or not.

If not I will  file a ticket to clean it up as a separate patch.
If it is Ok I will add it to the standard.
Please chime in.
  
_______________________________________________
sssd-devel mailing list
sssd-devel at lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

-- 
Thank you,
Dmitri Pal

Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/




More information about the sssd-devel mailing list