W dniu 08.04.2020 o 19:47, Thomas Monjalon pisze: > 08/04/2020 18:15, Lukasz Wojciechowski: >> Hi Thomas, >> >> Before my patch there was just a definition: >> #define RTE_TEST_TRACE_FAILURE TEST_TRACE_FAILURE >> without #ifndef condition. >> >> It caused a build problem to me when working on security test, which >> uses both rte_test.h and test.h >> As libraries should go first on the include list before local files I used: >> >> #include <rte_test.h> >> #include "test.h" >> >> sequence, which cause obvious build error as RTE_TEST_TRACE_FAILURE was >> first defined as an empty macro inside rte_test.h, and redefinition in >> test.h caused a problem. >> >> >> So I had two ways to solve the issue: >> 1) to wrap it with #ifndef condition and leave the definition there >> 2) to remove the redefinition from test.h >> >> I've chosen the 1) solution because: >> * Author of the former patch had placed the definition there for some >> purpose > Because rte_test.h is more recent and its addition was not complete enough. > rte_test.h should be included in test.h, and overlaps removed. > >> * In my opinion it is better to have the definition present and pointing >> to the same macro for both RTE_TEST_TRACE_FAILURE and TEST_TRACE_FAILURE >> as it would make logs look more consistent when printing information the >> same way. > I think solution 2 is better. Ok I'll change this patch and remove the macro definition at all from app/test/test.h I'll publish it with version 3, but I'll wait a bit more for getting more comments on version 2 > > > PS: please avoid top-posting Sorry > > >> W dniu 08.04.2020 o 14:53, Thomas Monjalon pisze: >>> 08/04/2020 05:13, Lukasz Wojciechowski: >>>> Wrap RTE_TEST_TRACE_FAILURE macro definition into #ifndef clause >>>> as it might be already defined. >>> I think it should not be defined at all. >>> Why not including rte_test.h? >>> >>> >>> > > > > > --
Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciec...@partner.samsung.com