On Mon, 2014-01-13 at 17:59 -0500, Dmitri Pal wrote: > On 01/13/2014 04:46 PM, Simo Sorce wrote: > > On Mon, 2014-01-13 at 15:00 -0500, Dmitri Pal wrote: > >> On 01/13/2014 02:53 PM, Stephen Gallagher wrote: > >>> On 01/13/2014 01:42 PM, Lukas Slebodnik wrote: > >>>> ehlo, > >>>> attached patch addresses ticket #2193 > >>>> I have just a question about refarray/libref_array.sym There is > >>>> extern function ref_array_debug, which is not defined in public > >>>> header file (not exposed in public API), but it is used in > >>>> ref_array unit tests. It needs to be in global section because > >>>> linker with fail to find symbol. Should I add any comment to the > >>>> file libref_array.sym or does anyone better solution? > >>> > >>> If this isn't a public API and isn't used except by the unit test, it > >>> should probably not be built as part of the library. It should > >>> probably be part of the unit test. > >> IMO this is conceptually wrong. > >> Unit test emulates external use of the library. Unit test uses only > >> public header and not private headers if any. > >> If we put debug function in the unit test we will expose internal of the > >> library that should not be know outside. > >> I always view unit tests as example of use. If we show that it is OK to > >> include private headers people would include private headers and try to > >> access internal structures directly. > > Sorry but I see a few issues with this argument. > > > > You say that unit tests only use the public API, the you proceed and > > use an undisclosed API that is not made available through the public > > header files ? Isn't this contradictory ? > > > > Also I see nothing wrong in accessing private data via unit tests, I > > never heard anyone have the expectation that unit tests are actually > > examples to follow. > > > >> Putting it into a separate module is probably the cleanest solution but > >> IMO too much overhead. > > A separate shared object would be too much, but you can statically link > > the function in the unit tests. > > > > I am not telling you how to resolve this issue, but the argument is a > > bit weak. > > > > Simo. > > > I agree that it is a bit weak. > It depends upon where you put the stake into the ground, I mean what is > the most important thing for an interface. > IMO the most important thing is for the interface to be clean and > consistent and not espose any internals. > The debug function spoils a perfect picture. On one hand it is not a > public function of the API on the other side it needs to be there for > the internals not to be seen outside the library. > So where it belongs in the library or outside? Since I assume the the > core principal is that "nothing knows about the internals of the library > other than the library itself" then the debug function is a part of the > library but is not included into the public header. It is a compromise > but a conscious compromise. And this approach you observe in the code. > You might agree or disagree since IMO this is a question of taste and > style rather than anything else.
I do not have a strong opinion, but I have a question. Why not simply expose the debug function in the public header then ? Simo. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
