| Message ID | 20191113004502.29986-7-takahiro.akashi@linaro.org |
|---|---|
| State | Accepted |
| Commit | bd3c3dd7fbb152412c16688cf3b70c6a302eda8a |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | import x509/pkcs7 parsers from linux | expand |
On 11/13/19 1:44 AM, AKASHI Takahiro wrote: > Without this commit, time.h possibly causes a build error as > asctime_r() uses sprintf(). asctime_r() is not a Linux symbol (as of next-20191119) ctime_r() and asctime_r() are defined as inline functions. ctime_r() is used in multiple places and so we may end up duplicating code. So I would prefer the inline functions in time.h to be moved to a separate C file in lib/. Best regards Heinrich > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > include/linux/time.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/linux/time.h b/include/linux/time.h > index dc9344a6d97b..702dd276aea5 100644 > --- a/include/linux/time.h > +++ b/include/linux/time.h > @@ -2,6 +2,7 @@ > #define _LINUX_TIME_H > > #include <rtc.h> > +#include <vsprintf.h> > #include <linux/types.h> > > #define _DEFUN(a,b,c) a(c) >
On 11/26/19 4:56 AM, Heinrich Schuchardt wrote: > On 11/13/19 1:44 AM, AKASHI Takahiro wrote: >> Without this commit, time.h possibly causes a build error as >> asctime_r() uses sprintf(). > > asctime_r() is not a Linux symbol (as of next-20191119) > > ctime_r() and asctime_r() are defined as inline functions. ctime_r() is > used in multiple places and so we may end up duplicating code. So I > would prefer the inline functions in time.h to be moved to a separate C > file in lib/. > > Best regards > > Heinrich Could it be that in one of your C files you simply didn't follow the U-Boot coding style convention to include common.h first and we don't need this patch at all? https://www.denx.de/wiki/U-Boot/CodingStyle#Include_files Best regards Heinrich
Heinrich, On Tue, Nov 26, 2019 at 08:31:18AM +0100, Heinrich Schuchardt wrote: > On 11/26/19 4:56 AM, Heinrich Schuchardt wrote: > >On 11/13/19 1:44 AM, AKASHI Takahiro wrote: > >>Without this commit, time.h possibly causes a build error as > >>asctime_r() uses sprintf(). > > > >asctime_r() is not a Linux symbol (as of next-20191119) I simply don't get your point here. > > > >ctime_r() and asctime_r() are defined as inline functions. ctime_r() is > >used in multiple places and so we may end up duplicating code. So I > >would prefer the inline functions in time.h to be moved to a separate C > >file in lib/. Basically I'm reluctant to do so. I have never touched ctime_r() nor asctime_r() as they were introduced by Wolfgang in 2002. Since then, nobody complained. So why now? > >Best regards > > > >Heinrich > > Could it be that in one of your C files you simply didn't follow the > U-Boot coding style convention to include common.h first I will address this issue in general in the future. > and we don't > need this patch at all? I commented against this above. I believe that *hidden* dependency of include files should not be exposed to users. In this case, for example, any code that uses asctime_r() should not be bothered with such a dependency as the code doesn't know what functions asctime_r() internally uses or even if that function is inline or not. I also mentioned this in another thread in different words: https://lists.denx.de/pipermail/u-boot/2019-November/391959.html Thanks, -Takahiro Akashi > https://www.denx.de/wiki/U-Boot/CodingStyle#Include_files > > Best regards > > Heinrich
On Wed, Nov 13, 2019 at 09:44:52AM +0900, AKASHI Takahiro wrote: > Without this commit, time.h possibly causes a build error as > asctime_r() uses sprintf(). > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> Applied to u-boot/master, thanks!
diff --git a/include/linux/time.h b/include/linux/time.h index dc9344a6d97b..702dd276aea5 100644 --- a/include/linux/time.h +++ b/include/linux/time.h @@ -2,6 +2,7 @@ #define _LINUX_TIME_H #include <rtc.h> +#include <vsprintf.h> #include <linux/types.h> #define _DEFUN(a,b,c) a(c)
Without this commit, time.h possibly causes a build error as asctime_r() uses sprintf(). Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- include/linux/time.h | 1 + 1 file changed, 1 insertion(+)