* [PATCH 1/2] define daddr_t type @ 2019-06-07 5:14 Petr Vorel 2019-06-07 5:14 ` [PATCH 2/2] define mt_fileno and mt_fileno struct mtget members as mt_blkno Petr Vorel 2019-06-07 5:28 ` [PATCH 1/2] define daddr_t type Rich Felker 0 siblings, 2 replies; 20+ messages in thread From: Petr Vorel @ 2019-06-07 5:14 UTC (permalink / raw) To: musl; +Cc: Petr Vorel According to kernel sources only mips (and sparc which we don't support) defines daddr_t as long, other define int. Signed-off-by: Petr Vorel <petr.vorel@gmail.com> --- arch/mips/bits/alltypes.h.in | 2 ++ arch/mipsn32/bits/alltypes.h.in | 2 ++ include/alltypes.h.in | 1 + include/sys/types.h | 1 + 4 files changed, 6 insertions(+) diff --git a/arch/mips/bits/alltypes.h.in b/arch/mips/bits/alltypes.h.in index 66ca18ad..bd062a85 100644 --- a/arch/mips/bits/alltypes.h.in +++ b/arch/mips/bits/alltypes.h.in @@ -17,6 +17,8 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t; TYPEDEF long time_t; TYPEDEF long suseconds_t; +TYPEDEF long daddr_t; + TYPEDEF struct { union { int __i[9]; volatile int __vi[9]; unsigned __s[9]; } __u; } pthread_attr_t; TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t; TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile __p[6]; } __u; } mtx_t; diff --git a/arch/mipsn32/bits/alltypes.h.in b/arch/mipsn32/bits/alltypes.h.in index 66ca18ad..bd062a85 100644 --- a/arch/mipsn32/bits/alltypes.h.in +++ b/arch/mipsn32/bits/alltypes.h.in @@ -17,6 +17,8 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t; TYPEDEF long time_t; TYPEDEF long suseconds_t; +TYPEDEF long daddr_t; + TYPEDEF struct { union { int __i[9]; volatile int __vi[9]; unsigned __s[9]; } __u; } pthread_attr_t; TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t; TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile __p[6]; } __u; } mtx_t; diff --git a/include/alltypes.h.in b/include/alltypes.h.in index 4cc879b1..6ef6ebd4 100644 --- a/include/alltypes.h.in +++ b/include/alltypes.h.in @@ -18,6 +18,7 @@ TYPEDEF unsigned _Int64 uint64_t; TYPEDEF unsigned _Int64 u_int64_t; TYPEDEF unsigned _Int64 uintmax_t; +TYPEDEF int daddr_t; TYPEDEF unsigned mode_t; TYPEDEF unsigned _Reg nlink_t; TYPEDEF _Int64 off_t; diff --git a/include/sys/types.h b/include/sys/types.h index 75e489c5..c50d21c9 100644 --- a/include/sys/types.h +++ b/include/sys/types.h @@ -29,6 +29,7 @@ extern "C" { #define __NEED_clock_t #define __NEED_suseconds_t #define __NEED_blksize_t +#define __NEED_daddr_t #define __NEED_pthread_t #define __NEED_pthread_attr_t -- 2.21.0 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] define mt_fileno and mt_fileno struct mtget members as mt_blkno 2019-06-07 5:14 [PATCH 1/2] define daddr_t type Petr Vorel @ 2019-06-07 5:14 ` Petr Vorel 2019-06-07 5:31 ` Rich Felker 2019-06-07 5:28 ` [PATCH 1/2] define daddr_t type Rich Felker 1 sibling, 1 reply; 20+ messages in thread From: Petr Vorel @ 2019-06-07 5:14 UTC (permalink / raw) To: musl; +Cc: Petr Vorel following kernel definition, this fixes size on mips. Signed-off-by: Petr Vorel <petr.vorel@gmail.com> --- include/sys/mtio.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/sys/mtio.h b/include/sys/mtio.h index f16a529b..4dd4c548 100644 --- a/include/sys/mtio.h +++ b/include/sys/mtio.h @@ -54,8 +54,8 @@ struct mtget { long mt_dsreg; long mt_gstat; long mt_erreg; - int mt_fileno; - int mt_blkno; + daddr_t mt_fileno; + daddr_t mt_blkno; }; #define MT_ISUNKNOWN 0x01 -- 2.21.0 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] define mt_fileno and mt_fileno struct mtget members as mt_blkno 2019-06-07 5:14 ` [PATCH 2/2] define mt_fileno and mt_fileno struct mtget members as mt_blkno Petr Vorel @ 2019-06-07 5:31 ` Rich Felker 2019-06-07 5:42 ` Petr Vorel 0 siblings, 1 reply; 20+ messages in thread From: Rich Felker @ 2019-06-07 5:31 UTC (permalink / raw) To: musl On Fri, Jun 07, 2019 at 07:14:44AM +0200, Petr Vorel wrote: > following kernel definition, this fixes size on mips. > > Signed-off-by: Petr Vorel <petr.vorel@gmail.com> > --- > include/sys/mtio.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/sys/mtio.h b/include/sys/mtio.h > index f16a529b..4dd4c548 100644 > --- a/include/sys/mtio.h > +++ b/include/sys/mtio.h > @@ -54,8 +54,8 @@ struct mtget { > long mt_dsreg; > long mt_gstat; > long mt_erreg; > - int mt_fileno; > - int mt_blkno; > + daddr_t mt_fileno; > + daddr_t mt_blkno; > }; > > #define MT_ISUNKNOWN 0x01 > -- > 2.21.0 Can you explain what problem this is supposed to fix? It definitely needs further discussion to determine what the right way is, but that's impossible without knowing the problem you're trying to solve. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] define mt_fileno and mt_fileno struct mtget members as mt_blkno 2019-06-07 5:31 ` Rich Felker @ 2019-06-07 5:42 ` Petr Vorel 2019-06-07 5:50 ` Rich Felker 0 siblings, 1 reply; 20+ messages in thread From: Petr Vorel @ 2019-06-07 5:42 UTC (permalink / raw) To: musl Hi Rich, ... > > +++ b/include/sys/mtio.h > > @@ -54,8 +54,8 @@ struct mtget { > > long mt_dsreg; > > long mt_gstat; > > long mt_erreg; > > - int mt_fileno; > > - int mt_blkno; > > + daddr_t mt_fileno; > > + daddr_t mt_blkno; ... > Can you explain what problem this is supposed to fix? It definitely > needs further discussion to determine what the right way is, but > that's impossible without knowing the problem you're trying to solve. Thanks for review. Not a problem for me actually. I've noticed, that glibc (and thus uclibc-ng) and bionic follow kernel sources, which defines it long for mips (and for sparc, but musl doesn't support it). Default is int [2]. Drop this patch if this is not an issue. > Rich Kind regards, Petr [1] https://elixir.bootlin.com/linux/latest/source/arch/mips/include/uapi/asm/posix_types.h#L21 [2] https://elixir.bootlin.com/linux/latest/source/include/uapi/asm-generic/posix_types.h#L45 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] define mt_fileno and mt_fileno struct mtget members as mt_blkno 2019-06-07 5:42 ` Petr Vorel @ 2019-06-07 5:50 ` Rich Felker 2019-06-07 5:58 ` Petr Vorel 0 siblings, 1 reply; 20+ messages in thread From: Rich Felker @ 2019-06-07 5:50 UTC (permalink / raw) To: musl On Fri, Jun 07, 2019 at 07:42:15AM +0200, Petr Vorel wrote: > Hi Rich, > > .... > > > +++ b/include/sys/mtio.h > > > @@ -54,8 +54,8 @@ struct mtget { > > > long mt_dsreg; > > > long mt_gstat; > > > long mt_erreg; > > > - int mt_fileno; > > > - int mt_blkno; > > > + daddr_t mt_fileno; > > > + daddr_t mt_blkno; > .... > > > Can you explain what problem this is supposed to fix? It definitely > > needs further discussion to determine what the right way is, but > > that's impossible without knowing the problem you're trying to solve. > Thanks for review. Not a problem for me actually. I've noticed, that glibc (and > thus uclibc-ng) and bionic follow kernel sources, which defines it long for mips > (and for sparc, but musl doesn't support it). Default is int [2]. Drop this > patch if this is not an issue. Well it might be a problem on some archs. I think it's worth looking into. We might need to add bits/mtio.h to define it appropriately for the arch. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] define mt_fileno and mt_fileno struct mtget members as mt_blkno 2019-06-07 5:50 ` Rich Felker @ 2019-06-07 5:58 ` Petr Vorel 2019-06-07 6:01 ` Rich Felker 0 siblings, 1 reply; 20+ messages in thread From: Petr Vorel @ 2019-06-07 5:58 UTC (permalink / raw) To: musl Hi Rich, > > .... > > > > +++ b/include/sys/mtio.h > > > > @@ -54,8 +54,8 @@ struct mtget { > > > > long mt_dsreg; > > > > long mt_gstat; > > > > long mt_erreg; > > > > - int mt_fileno; > > > > - int mt_blkno; > > > > + daddr_t mt_fileno; > > > > + daddr_t mt_blkno; > > .... > > > Can you explain what problem this is supposed to fix? It definitely > > > needs further discussion to determine what the right way is, but > > > that's impossible without knowing the problem you're trying to solve. > > Thanks for review. Not a problem for me actually. I've noticed, that glibc (and > > thus uclibc-ng) and bionic follow kernel sources, which defines it long for mips > > (and for sparc, but musl doesn't support it). Default is int [2]. Drop this > > patch if this is not an issue. > Well it might be a problem on some archs. I think it's worth looking > into. We might need to add bits/mtio.h to define it appropriately for > the arch. I'd be for bits/mtio.h as I suppose this problem still persists on mips. But I'm not able to verify, just follow what it's defined in kernel/glibc/uclibc/bionic. My main concern is to have daddr_t defined in musl. > Rich Kind regards, Petr ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] define mt_fileno and mt_fileno struct mtget members as mt_blkno 2019-06-07 5:58 ` Petr Vorel @ 2019-06-07 6:01 ` Rich Felker 0 siblings, 0 replies; 20+ messages in thread From: Rich Felker @ 2019-06-07 6:01 UTC (permalink / raw) To: musl On Fri, Jun 07, 2019 at 07:58:29AM +0200, Petr Vorel wrote: > Hi Rich, > > > > .... > > > > > +++ b/include/sys/mtio.h > > > > > @@ -54,8 +54,8 @@ struct mtget { > > > > > long mt_dsreg; > > > > > long mt_gstat; > > > > > long mt_erreg; > > > > > - int mt_fileno; > > > > > - int mt_blkno; > > > > > + daddr_t mt_fileno; > > > > > + daddr_t mt_blkno; > > > .... > > > > > Can you explain what problem this is supposed to fix? It definitely > > > > needs further discussion to determine what the right way is, but > > > > that's impossible without knowing the problem you're trying to solve. > > > Thanks for review. Not a problem for me actually. I've noticed, that glibc (and > > > thus uclibc-ng) and bionic follow kernel sources, which defines it long for mips > > > (and for sparc, but musl doesn't support it). Default is int [2]. Drop this > > > patch if this is not an issue. > > > Well it might be a problem on some archs. I think it's worth looking > > into. We might need to add bits/mtio.h to define it appropriately for > > the arch. > I'd be for bits/mtio.h as I suppose this problem still persists on mips. > But I'm not able to verify, just follow what it's defined in > kernel/glibc/uclibc/bionic. > > My main concern is to have daddr_t defined in musl. I don't follow why this is your goal/concern. It's not a standard type, but some historical baggage from the 70s or 80s that some implementations carried forward. It doesn't seem to meet any of musl's criteria for inclusion. If you think this is incorrect, please explain why you think it's important to have it rather than just assuming it is. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 5:14 [PATCH 1/2] define daddr_t type Petr Vorel 2019-06-07 5:14 ` [PATCH 2/2] define mt_fileno and mt_fileno struct mtget members as mt_blkno Petr Vorel @ 2019-06-07 5:28 ` Rich Felker 2019-06-07 5:53 ` Petr Vorel 1 sibling, 1 reply; 20+ messages in thread From: Rich Felker @ 2019-06-07 5:28 UTC (permalink / raw) To: musl On Fri, Jun 07, 2019 at 07:14:43AM +0200, Petr Vorel wrote: > According to kernel sources only mips (and sparc which we don't support) > defines daddr_t as long, other define int. > > Signed-off-by: Petr Vorel <petr.vorel@gmail.com> > --- > arch/mips/bits/alltypes.h.in | 2 ++ > arch/mipsn32/bits/alltypes.h.in | 2 ++ > include/alltypes.h.in | 1 + > include/sys/types.h | 1 + > 4 files changed, 6 insertions(+) > > diff --git a/arch/mips/bits/alltypes.h.in b/arch/mips/bits/alltypes.h.in > index 66ca18ad..bd062a85 100644 > --- a/arch/mips/bits/alltypes.h.in > +++ b/arch/mips/bits/alltypes.h.in > @@ -17,6 +17,8 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t; > TYPEDEF long time_t; > TYPEDEF long suseconds_t; > > +TYPEDEF long daddr_t; > + > TYPEDEF struct { union { int __i[9]; volatile int __vi[9]; unsigned __s[9]; } __u; } pthread_attr_t; > TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t; > TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile __p[6]; } __u; } mtx_t; > diff --git a/arch/mipsn32/bits/alltypes.h.in b/arch/mipsn32/bits/alltypes.h.in > index 66ca18ad..bd062a85 100644 > --- a/arch/mipsn32/bits/alltypes.h.in > +++ b/arch/mipsn32/bits/alltypes.h.in > @@ -17,6 +17,8 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t; > TYPEDEF long time_t; > TYPEDEF long suseconds_t; > > +TYPEDEF long daddr_t; > + > TYPEDEF struct { union { int __i[9]; volatile int __vi[9]; unsigned __s[9]; } __u; } pthread_attr_t; > TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t; > TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile __p[6]; } __u; } mtx_t; > diff --git a/include/alltypes.h.in b/include/alltypes.h.in > index 4cc879b1..6ef6ebd4 100644 > --- a/include/alltypes.h.in > +++ b/include/alltypes.h.in > @@ -18,6 +18,7 @@ TYPEDEF unsigned _Int64 uint64_t; > TYPEDEF unsigned _Int64 u_int64_t; > TYPEDEF unsigned _Int64 uintmax_t; > > +TYPEDEF int daddr_t; > TYPEDEF unsigned mode_t; > TYPEDEF unsigned _Reg nlink_t; > TYPEDEF _Int64 off_t; > diff --git a/include/sys/types.h b/include/sys/types.h > index 75e489c5..c50d21c9 100644 > --- a/include/sys/types.h > +++ b/include/sys/types.h > @@ -29,6 +29,7 @@ extern "C" { > #define __NEED_clock_t > #define __NEED_suseconds_t > #define __NEED_blksize_t > +#define __NEED_daddr_t > > #define __NEED_pthread_t > #define __NEED_pthread_attr_t > -- > 2.21.0 daddr_t is not a standard type, so can't be exposed by default here (aside from the dubious "*_t is always reserved" rule), and it's only proposed to be used in one header, so it doesn't belong in alltypes.h either. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 5:28 ` [PATCH 1/2] define daddr_t type Rich Felker @ 2019-06-07 5:53 ` Petr Vorel 2019-06-07 5:58 ` Rich Felker 0 siblings, 1 reply; 20+ messages in thread From: Petr Vorel @ 2019-06-07 5:53 UTC (permalink / raw) To: musl; +Cc: Rich Felker Hi Rich, ... > > +++ b/include/sys/types.h > > @@ -29,6 +29,7 @@ extern "C" { > > #define __NEED_clock_t > > #define __NEED_suseconds_t > > #define __NEED_blksize_t > > +#define __NEED_daddr_t ... > daddr_t is not a standard type, so can't be exposed by default here > (aside from the dubious "*_t is always reserved" rule), and it's only So should it be wrapped by #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE) ? > proposed to be used in one header, so it doesn't belong in alltypes.h > either. Where should it be then? Shell I create bits/types.h for it? The goal is to be loadable from <sys/types.h> > Rich Kind regards, Petr ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 5:53 ` Petr Vorel @ 2019-06-07 5:58 ` Rich Felker 2019-06-07 6:06 ` Petr Vorel 2019-06-07 12:52 ` Leah Neukirchen 0 siblings, 2 replies; 20+ messages in thread From: Rich Felker @ 2019-06-07 5:58 UTC (permalink / raw) To: musl On Fri, Jun 07, 2019 at 07:53:29AM +0200, Petr Vorel wrote: > Hi Rich, > > .... > > > +++ b/include/sys/types.h > > > @@ -29,6 +29,7 @@ extern "C" { > > > #define __NEED_clock_t > > > #define __NEED_suseconds_t > > > #define __NEED_blksize_t > > > +#define __NEED_daddr_t > .... > > > daddr_t is not a standard type, so can't be exposed by default here > > (aside from the dubious "*_t is always reserved" rule), and it's only > So should it be wrapped by #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE) ? > > > proposed to be used in one header, so it doesn't belong in alltypes.h > > either. > Where should it be then? Shell I create bits/types.h for it? > The goal is to be loadable from <sys/types.h> Why? It's not a reasonable type for any application to use -- we've never gotten a report that something failed to build because of its absence, and even if we did, it would almost surely be a case of "fix the application". It looks like the only reason you wanted it was to fix the type of a field in an mtio structure, and in that case the type would just need to be defined in mtio.h. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 5:58 ` Rich Felker @ 2019-06-07 6:06 ` Petr Vorel 2019-06-07 6:18 ` Rich Felker 2019-06-07 12:52 ` Leah Neukirchen 1 sibling, 1 reply; 20+ messages in thread From: Petr Vorel @ 2019-06-07 6:06 UTC (permalink / raw) To: musl Hi Rich, > > .... > > > > +++ b/include/sys/types.h > > > > @@ -29,6 +29,7 @@ extern "C" { > > > > #define __NEED_clock_t > > > > #define __NEED_suseconds_t > > > > #define __NEED_blksize_t > > > > +#define __NEED_daddr_t > > .... > > > daddr_t is not a standard type, so can't be exposed by default here > > > (aside from the dubious "*_t is always reserved" rule), and it's only > > So should it be wrapped by #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE) ? > > > proposed to be used in one header, so it doesn't belong in alltypes.h > > > either. > > Where should it be then? Shell I create bits/types.h for it? > > The goal is to be loadable from <sys/types.h> > Why? It's not a reasonable type for any application to use -- we've > never gotten a report that something failed to build because of its > absence, and even if we did, it would almost surely be a case of "fix > the application". It looks like the only reason you wanted it was to > fix the type of a field in an mtio structure, and in that case the > type would just need to be defined in mtio.h. I need it for LTP [1]. It's actually workaround for missing struct ustat [2]. If it's really useless to have it in musl, I'll use in LTP __kernel_daddr_t from <linux/types.h>. [1] https://patchwork.ozlabs.org/patch/1102380/ [2] https://github.com/linux-test-project/ltp/blob/master/include/lapi/ustat.h > Rich Kind regards, Petr ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 6:06 ` Petr Vorel @ 2019-06-07 6:18 ` Rich Felker 2019-06-07 6:28 ` Petr Vorel 0 siblings, 1 reply; 20+ messages in thread From: Rich Felker @ 2019-06-07 6:18 UTC (permalink / raw) To: musl On Fri, Jun 07, 2019 at 08:06:20AM +0200, Petr Vorel wrote: > Hi Rich, > > > > .... > > > > > +++ b/include/sys/types.h > > > > > @@ -29,6 +29,7 @@ extern "C" { > > > > > #define __NEED_clock_t > > > > > #define __NEED_suseconds_t > > > > > #define __NEED_blksize_t > > > > > +#define __NEED_daddr_t > > > .... > > > > > daddr_t is not a standard type, so can't be exposed by default here > > > > (aside from the dubious "*_t is always reserved" rule), and it's only > > > So should it be wrapped by #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE) ? > > > > > proposed to be used in one header, so it doesn't belong in alltypes.h > > > > either. > > > Where should it be then? Shell I create bits/types.h for it? > > > The goal is to be loadable from <sys/types.h> > > > Why? It's not a reasonable type for any application to use -- we've > > never gotten a report that something failed to build because of its > > absence, and even if we did, it would almost surely be a case of "fix > > the application". It looks like the only reason you wanted it was to > > fix the type of a field in an mtio structure, and in that case the > > type would just need to be defined in mtio.h. > > I need it for LTP [1]. It's actually workaround for missing struct ustat [2]. > If it's really useless to have it in musl, I'll use in LTP __kernel_daddr_t from <linux/types.h>. > > [1] https://patchwork.ozlabs.org/patch/1102380/ > [2] https://github.com/linux-test-project/ltp/blob/master/include/lapi/ustat.h Per http://man7.org/linux/man-pages/man2/ustat.2.html: ustat() is deprecated and has been provided only for compatibility. All new programs should use statfs(2) instead. AFAIK we've never hit anything that needed/wanted ustat, and the interface has fundamental limitations that make it mostly (entirely?) non-useful. I'm not sure why LTP is testing it... Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 6:18 ` Rich Felker @ 2019-06-07 6:28 ` Petr Vorel 2019-06-07 7:22 ` Florian Weimer 0 siblings, 1 reply; 20+ messages in thread From: Petr Vorel @ 2019-06-07 6:28 UTC (permalink / raw) To: musl Hi Rich, ... > Per http://man7.org/linux/man-pages/man2/ustat.2.html: > ustat() is deprecated and has been provided only for > compatibility. All new programs should use statfs(2) instead. > AFAIK we've never hit anything that needed/wanted ustat, and the > interface has fundamental limitations that make it mostly (entirely?) > non-useful. I'm not sure why LTP is testing it... Thanks for info. I'll read man page properly next time :). LTP tests what is in glibc (will test ustat() until it's removed from glibc). I guess fix for mtio.h is still valid, I'll send a patch. > Rich Kind regards, Petr ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 6:28 ` Petr Vorel @ 2019-06-07 7:22 ` Florian Weimer 2019-06-07 7:48 ` Petr Vorel 0 siblings, 1 reply; 20+ messages in thread From: Florian Weimer @ 2019-06-07 7:22 UTC (permalink / raw) To: Petr Vorel; +Cc: musl * Petr Vorel: > LTP tests what is in glibc (will test ustat() until it's removed from glibc). > I guess fix for mtio.h is still valid, I'll send a patch. ustat was removed from glibc 2.28. Therefore, it's missing from the csky port, which was added in glibc 2.29. Thanks, Florian ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 7:22 ` Florian Weimer @ 2019-06-07 7:48 ` Petr Vorel 2019-06-07 9:21 ` Petr Vorel 0 siblings, 1 reply; 20+ messages in thread From: Petr Vorel @ 2019-06-07 7:48 UTC (permalink / raw) To: Florian Weimer; +Cc: musl Hi Florian, > * Petr Vorel: > > LTP tests what is in glibc (will test ustat() until it's removed from glibc). > > I guess fix for mtio.h is still valid, I'll send a patch. > ustat was removed from glibc 2.28. Therefore, it's missing from the > csky port, which was added in glibc 2.29. Thanks! Another reason to add autotools check to LTP. > Thanks, > Florian Kind regards, Petr ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 7:48 ` Petr Vorel @ 2019-06-07 9:21 ` Petr Vorel 0 siblings, 0 replies; 20+ messages in thread From: Petr Vorel @ 2019-06-07 9:21 UTC (permalink / raw) To: Florian Weimer; +Cc: musl Hi Florian, > > > LTP tests what is in glibc (will test ustat() until it's removed from glibc). > > > I guess fix for mtio.h is still valid, I'll send a patch. > > ustat was removed from glibc 2.28. Therefore, it's missing from the > > csky port, which was added in glibc 2.29. > Thanks! Another reason to add autotools check to LTP. Actually LTP tests __NR_ustat syscall. But still needs the structure. But in that case it makes sense to use __kernel_daddr_t (which was internally used by other libc implementations anyway. Kind regards, Petr ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 5:58 ` Rich Felker 2019-06-07 6:06 ` Petr Vorel @ 2019-06-07 12:52 ` Leah Neukirchen 2019-06-07 15:25 ` Rich Felker 2019-06-07 20:57 ` Petr Vorel 1 sibling, 2 replies; 20+ messages in thread From: Leah Neukirchen @ 2019-06-07 12:52 UTC (permalink / raw) To: Rich Felker; +Cc: musl Rich Felker <dalias@libc.org> writes: > On Fri, Jun 07, 2019 at 07:53:29AM +0200, Petr Vorel wrote: >> Hi Rich, >> >> .... >> > > +++ b/include/sys/types.h >> > > @@ -29,6 +29,7 @@ extern "C" { >> > > #define __NEED_clock_t >> > > #define __NEED_suseconds_t >> > > #define __NEED_blksize_t >> > > +#define __NEED_daddr_t >> .... >> >> > daddr_t is not a standard type, so can't be exposed by default here >> > (aside from the dubious "*_t is always reserved" rule), and it's only >> So should it be wrapped by #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE) ? >> >> > proposed to be used in one header, so it doesn't belong in alltypes.h >> > either. >> Where should it be then? Shell I create bits/types.h for it? >> The goal is to be loadable from <sys/types.h> > > Why? It's not a reasonable type for any application to use -- we've > never gotten a report that something failed to build because of its > absence, and even if we did, it would almost surely be a case of "fix > the application". FWIW, Void patches four packages that use daddr_t: - gpart - kpartx - libtirpc - sleuthkit Additionally, five packages use caddr_t: - ifenslave - lwipv6 - macchanger - sbcl - virtuoso -- Leah Neukirchen <leah@vuxu.org> http://leahneukirchen.org/ ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 12:52 ` Leah Neukirchen @ 2019-06-07 15:25 ` Rich Felker 2019-06-10 20:30 ` Rich Felker 2019-06-07 20:57 ` Petr Vorel 1 sibling, 1 reply; 20+ messages in thread From: Rich Felker @ 2019-06-07 15:25 UTC (permalink / raw) To: musl On Fri, Jun 07, 2019 at 02:52:07PM +0200, Leah Neukirchen wrote: > Rich Felker <dalias@libc.org> writes: > > > On Fri, Jun 07, 2019 at 07:53:29AM +0200, Petr Vorel wrote: > >> Hi Rich, > >> > >> .... > >> > > +++ b/include/sys/types.h > >> > > @@ -29,6 +29,7 @@ extern "C" { > >> > > #define __NEED_clock_t > >> > > #define __NEED_suseconds_t > >> > > #define __NEED_blksize_t > >> > > +#define __NEED_daddr_t > >> .... > >> > >> > daddr_t is not a standard type, so can't be exposed by default here > >> > (aside from the dubious "*_t is always reserved" rule), and it's only > >> So should it be wrapped by #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE) ? > >> > >> > proposed to be used in one header, so it doesn't belong in alltypes.h > >> > either. > >> Where should it be then? Shell I create bits/types.h for it? > >> The goal is to be loadable from <sys/types.h> > > > > Why? It's not a reasonable type for any application to use -- we've > > never gotten a report that something failed to build because of its > > absence, and even if we did, it would almost surely be a case of "fix > > the application". > > FWIW, Void patches four packages that use daddr_t: > > - gpart > - kpartx > - libtirpc > - sleuthkit > > Additionally, five packages use caddr_t: > > - ifenslave > - lwipv6 > - macchanger > - sbcl > - virtuoso Thanks. Can you tell if they have any legitimate motivation for doing so? What even is the conceptual definition of daddr_t? AIUI, caddr_t was historically something like void * or uintptr_t, a type for abstract addresses that might not actually point to objects, that mainly existed because void * didn't yet exist, but I have no idea what daddr_t is supposed to be. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 15:25 ` Rich Felker @ 2019-06-10 20:30 ` Rich Felker 0 siblings, 0 replies; 20+ messages in thread From: Rich Felker @ 2019-06-10 20:30 UTC (permalink / raw) To: musl On Fri, Jun 07, 2019 at 11:25:45AM -0400, Rich Felker wrote: > On Fri, Jun 07, 2019 at 02:52:07PM +0200, Leah Neukirchen wrote: > > Rich Felker <dalias@libc.org> writes: > > > > > On Fri, Jun 07, 2019 at 07:53:29AM +0200, Petr Vorel wrote: > > >> Hi Rich, > > >> > > >> .... > > >> > > +++ b/include/sys/types.h > > >> > > @@ -29,6 +29,7 @@ extern "C" { > > >> > > #define __NEED_clock_t > > >> > > #define __NEED_suseconds_t > > >> > > #define __NEED_blksize_t > > >> > > +#define __NEED_daddr_t > > >> .... > > >> > > >> > daddr_t is not a standard type, so can't be exposed by default here > > >> > (aside from the dubious "*_t is always reserved" rule), and it's only > > >> So should it be wrapped by #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE) ? > > >> > > >> > proposed to be used in one header, so it doesn't belong in alltypes.h > > >> > either. > > >> Where should it be then? Shell I create bits/types.h for it? > > >> The goal is to be loadable from <sys/types.h> > > > > > > Why? It's not a reasonable type for any application to use -- we've > > > never gotten a report that something failed to build because of its > > > absence, and even if we did, it would almost surely be a case of "fix > > > the application". > > > > FWIW, Void patches four packages that use daddr_t: > > > > - gpart > > - kpartx > > - libtirpc > > - sleuthkit > > > > Additionally, five packages use caddr_t: > > > > - ifenslave > > - lwipv6 > > - macchanger > > - sbcl > > - virtuoso > > Thanks. Can you tell if they have any legitimate motivation for doing > so? What even is the conceptual definition of daddr_t? AIUI, caddr_t > was historically something like void * or uintptr_t, a type for > abstract addresses that might not actually point to objects, that > mainly existed because void * didn't yet exist, but I have no idea > what daddr_t is supposed to be. OK, some Google-fu turns up that the historical meaning of daddr_t was "disk address" (as opposed to caddr_t which AIUI means "core address"). In that light, defining it would be completely wrong, since the definition should match off_t (long long), not int/long, but changing it to match would break any actual historical use of it... This should probably be treated as a bug against the mtio interface definition... Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] define daddr_t type 2019-06-07 12:52 ` Leah Neukirchen 2019-06-07 15:25 ` Rich Felker @ 2019-06-07 20:57 ` Petr Vorel 1 sibling, 0 replies; 20+ messages in thread From: Petr Vorel @ 2019-06-07 20:57 UTC (permalink / raw) To: musl; +Cc: Rich Felker, Leah Neukirchen > Rich Felker <dalias@libc.org> writes: > > On Fri, Jun 07, 2019 at 07:53:29AM +0200, Petr Vorel wrote: > >> Hi Rich, > >> .... > >> > > +++ b/include/sys/types.h > >> > > @@ -29,6 +29,7 @@ extern "C" { > >> > > #define __NEED_clock_t > >> > > #define __NEED_suseconds_t > >> > > #define __NEED_blksize_t > >> > > +#define __NEED_daddr_t > >> .... > >> > daddr_t is not a standard type, so can't be exposed by default here > >> > (aside from the dubious "*_t is always reserved" rule), and it's only > >> So should it be wrapped by #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE) ? > >> > proposed to be used in one header, so it doesn't belong in alltypes.h > >> > either. > >> Where should it be then? Shell I create bits/types.h for it? > >> The goal is to be loadable from <sys/types.h> > > Why? It's not a reasonable type for any application to use -- we've > > never gotten a report that something failed to build because of its > > absence, and even if we did, it would almost surely be a case of "fix > > the application". > FWIW, Void patches four packages that use daddr_t: > - gpart UPSTREAM GIT: some solaris related feature (compiled for linux) [1] struct solaris_x86_slice { ushort s_tag; /* ID tag of partition */ ushort s_flag; /* permision flags */ daddr_t s_start; /* start sector no of partition */ long s_size; /* # of blocks in partition */ }; Void: use int [2]. > - kpartx UPSTREAM GIT: the same thing, but daddr_t out, they decided to use long [3] //typedef int daddr_t; /* or long - check */ struct solaris_x86_slice { unsigned short s_tag; /* ID tag of partition */ unsigned short s_flag; /* permission flags */ long s_start; /* start sector no of partition */ long s_size; /* # of blocks in partition */ }; Void: use uint32_t [4]. daddr_t in these two is at least used. In the others it's not: > - libtirpc UPSTREAM GIT: daddr_t is defined [5], but I don't see it used anywhere. But caddr_t is used heavily. Unfortunately libtirpc upstream is not responding much, if anyone wants to remove daddr_t in upstream. Void: enables it, but IMHO could be removed. > - sleuthkit UPSTREAM GIT: Again, defined [7], but not used. Void: use uint32_t [8]. Kind regards, Petr [1] https://github.com/baruch/gpart/blob/master/src/gm_s86dl.h#L43 [2] https://github.com/void-linux/void-packages/blob/master/srcpkgs/gpart/patches/musl_loff_t.patch [3] https://git.opensvc.com/gitweb.cgi?p=multipath-tools/.git;a=blob;f=kpartx/solaris.c;h=8c1a971be698675d3c99ef83a5958bc27099c73a;hb=HEAD [4] https://github.com/void-linux/void-packages/blob/master/srcpkgs/kpartx/template#L20 [5] http://git.linux-nfs.org/?p=steved/libtirpc.git;a=blob;f=tirpc/rpc/types.h;h=f069efa77d1da598eba99d637107f971036ca535;hb=HEAD#l85 [6] https://github.com/void-linux/void-packages/blob/master/srcpkgs/libtirpc/patches/musl.patch [7] https://github.com/sleuthkit/sleuthkit/blob/develop/tsk/base/tsk_os.h#L55 [8] https://github.com/void-linux/void-packages/blob/master/srcpkgs/sleuthkit/template ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-06-10 20:30 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-07 5:14 [PATCH 1/2] define daddr_t type Petr Vorel 2019-06-07 5:14 ` [PATCH 2/2] define mt_fileno and mt_fileno struct mtget members as mt_blkno Petr Vorel 2019-06-07 5:31 ` Rich Felker 2019-06-07 5:42 ` Petr Vorel 2019-06-07 5:50 ` Rich Felker 2019-06-07 5:58 ` Petr Vorel 2019-06-07 6:01 ` Rich Felker 2019-06-07 5:28 ` [PATCH 1/2] define daddr_t type Rich Felker 2019-06-07 5:53 ` Petr Vorel 2019-06-07 5:58 ` Rich Felker 2019-06-07 6:06 ` Petr Vorel 2019-06-07 6:18 ` Rich Felker 2019-06-07 6:28 ` Petr Vorel 2019-06-07 7:22 ` Florian Weimer 2019-06-07 7:48 ` Petr Vorel 2019-06-07 9:21 ` Petr Vorel 2019-06-07 12:52 ` Leah Neukirchen 2019-06-07 15:25 ` Rich Felker 2019-06-10 20:30 ` Rich Felker 2019-06-07 20:57 ` Petr Vorel
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).