Re: [PATCH 1/2] define daddr_t type

[Petr Vorel <pvorel@suse.cz>]

> 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