On Tue, Sep 03, 2013 at 04:33:03PM +0200, Lukas Slebodnik wrote: > On (03/09/13 12:15), Sumit Bose wrote: > >On Mon, Sep 02, 2013 at 07:04:22PM +0200, Lukas Slebodnik wrote: > >> ehlo > >> > >> Patches are attached. > > > >I haven't tested to patches so far but already have some comments. > > > >bye, > >Sumit > > > >> > >> LS > > > >> From 6e3b789f4b24198b2ec4b40fb09e8b97e578044a Mon Sep 17 00:00:00 2001 > >> From: Lukas Slebodnik <[email protected]> > >> Date: Sat, 31 Aug 2013 01:12:01 +0200 > >> Subject: [PATCH 2/9] AUTOTOOLS: add check for type intptr_t > >> > >> We check whether HAVE_INTPTR_T is defined in definition of macro > >> discard_const_p, but autootols macro AC_CHECK_TYPE did not generate it. > >> --- > >> src/external/sizes.m4 | 3 +-- > >> 1 file changed, 1 insertion(+), 2 deletions(-) > >> > >> diff --git a/src/external/sizes.m4 b/src/external/sizes.m4 > >> index > >> 53df61dedc3ae748c50ca9a3935632087834155d..1018d846016541043d81fb2a53609ad9c562071a > >> 100644 > >> --- a/src/external/sizes.m4 > >> +++ b/src/external/sizes.m4 > >> @@ -37,8 +37,7 @@ AC_CHECK_SIZEOF(off_t) > >> AC_CHECK_SIZEOF(size_t) > >> AC_CHECK_SIZEOF(ssize_t) > >> > >> +AC_CHECK_TYPES([intptr_t]) > >> AC_CHECK_TYPE(intptr_t, long long) > >> AC_CHECK_TYPE(uintptr_t, unsigned long long) > >> AC_CHECK_TYPE(ptrdiff_t, unsigned long long) > > > >I guess we missed the change of AC_CHECK_TYPE in Autoconf 2.14 where the > >style used above still worked. If I understand the docs correctly it > >should still work, but you should remove the old AC_CHECK_TYPE for > >intptr_t. > > just a comment to this point. > > It may be litle bit confusing. > There are 3 macros with similar names. > AC_CHECK_TYPE (type, [action-if-found], [action-if-not-found], > [includes = ‘AC_INCLUDES_DEFAULT’]) > AC_CHECK_TYPES (types, [action-if-found], [action-if-not-found], > [includes = ‘AC_INCLUDES_DEFAULT’]) > And obsolete macro: > AC_CHECK_TYPE (type, default) > --If the type type is not defined, define it to be the C (or C++) builtin > type default, e.g., ‘short int’ or ‘unsigned int’. > --missing types are defined using #define and not typedef > > I hope we used obsolete version.
Yes we did. > There is a patter how to rewrite it using > new macro. We can ged rid of them. I'm always in favor of getting rid of obsolete stuff, but I don't think we need to do it now (unless it's trivial). > > And now about non obsolete versions. > > There is a difference between AC_CHECK_TYPE and AC_CHECK_TYPES > The second version: For each type of the types that is defined, > define HAVE_type > > >> +AC_CHECK_TYPES([intptr_t]) > -> this generate HAVE_INTPTR_T if exists intptr_t > >> AC_CHECK_TYPE(intptr_t, long long) > -> this generate "#define intptr_t longlong" unless exists intptr_t Hm, I wonder if there are any systems out there that depend on the compat #define generated by AC_CHECK_TYPE. Perhaps if we get rid of the macros, we should do it in backwards-compatible way. > > And here is how discard_const_p is defined. > > #ifndef discard_const_p > #if defined(__intptr_t_defined) || defined(HAVE_INTPTR_T) > ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^ > This is not portable This was not generated by autoconf > # define discard_const_p(type, ptr) ((type *)((intptr_t)(ptr))) > #else > # define discard_const_p(type, ptr) ((type *)(ptr)) > #endif > #endif > > If intptr_t is not default(builtin) type, we will define it in macro > AC_CHECK_TYPE (new one or obsolete). > Which branch of discard_const_p should be used? > I would prefer the portable branch guarded by defined. > I need to know how to use macro. > > And the second question. Do wee need defined HAVE_type for all types? > (only HAVE_INTPTR_T is used in code) I think it would be more defensive but not strictly needed. If we had the macro then we could always use HAVE_$type and not worry about forgetting that some types have the HAVE_$type definition and some not. But then again, it wasn't an issue until now. _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
