-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 09/04/2009 12:38 PM, Dmitri Pal wrote: > 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@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@lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/sssd-devel >
My personal preference is to not use typedefs OR defines, and instead rely on using the 'struct <name>' approach. Aliasing variable types is always confusing to someone reading the code. The only exception I feel is warranted is typedef-ing a function pointer prototype, since otherwise they are just too long to be reasonably used throughout the code. - -- Stephen Gallagher RHCE 804006346421761 Looking to carve out IT costs? www.redhat.com/carveoutcosts/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkqhRf4ACgkQeiVVYja6o6P6jQCfV0wJmluh33KirHc0dg2a6+C0 odUAnjQvP//sQlJBhcFq5veWeb/h2NJW =XfGk -----END PGP SIGNATURE----- _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
