mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] AF_LOCAL
@ 2022-07-10 14:32 Tomasz Duda
  2022-07-10 18:06 ` Markus Wichmann
  2022-07-11  4:48 ` Florian Weimer
  0 siblings, 2 replies; 8+ messages in thread
From: Tomasz Duda @ 2022-07-10 14:32 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 1675 bytes --]

Hi,

I'm using
python:3.9-alpine3.14
It looks that AF_LOCAL is not implemented.
It would be nice if you could include it.

BR,
TD

diff --git a/src/network/getnameinfo.c b/src/network/getnameinfo.c
index 949e181..7d3a9c3 100644
--- a/src/network/getnameinfo.c
+++ b/src/network/getnameinfo.c
@@ -11,7 +11,9 @@
#include <resolv.h>
#include "lookup.h"
#include "stdio_impl.h"
+#include <sys/un.h>
+#define MIN(a,b) ((a)<(b) ? (a) : (b))
#define PTR_MAX (64 + sizeof ".in-addr.arpa")
#define RR_PTR 12
@@ -118,6 +120,29 @@ static int dns_parse_callback(void *c, int rr, const
void *data, int len, const
}
+/*
+ * getnameinfo_local():
+ * Format an local address into a printable format.
+ */
+/* ARGSUSED */
+static int
+getnameinfo_local(const struct sockaddr *sa, socklen_t salen,
+ char *host, socklen_t hostlen, char *serv, socklen_t servlen,
+ int flags __attribute__((unused)))
+{
+ const struct sockaddr_un *sun =
+ (const struct sockaddr_un *)(const void *)sa;
+ if (salen < (socklen_t) offsetof(struct sockaddr_un, sun_path)) {
+ return EAI_FAMILY;
+ }
+ if (serv != NULL && servlen > 0)
+ serv[0] = '\0';
+ if (host && hostlen > 0)
+ strncpy(host, sun->sun_path,
+ MIN((socklen_t) sizeof(sun->sun_path) + 1, hostlen));
+ return 0;
+}
+
int getnameinfo(const struct sockaddr *restrict sa, socklen_t sl,
char *restrict node, socklen_t nodelen,
char *restrict serv, socklen_t servlen,
@@ -145,6 +170,8 @@ int getnameinfo(const struct sockaddr *restrict sa,
socklen_t sl,
mkptr4(ptr, a+12);
scopeid = ((struct sockaddr_in6 *)sa)->sin6_scope_id;
break;
+ case AF_LOCAL:
+ return getnameinfo_local(sa, sl, node, nodelen, serv, servlen, flags);
default:
return EAI_FAMILY;
}

[-- Attachment #2: Type: text/html, Size: 5446 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] AF_LOCAL
  2022-07-10 14:32 [musl] AF_LOCAL Tomasz Duda
@ 2022-07-10 18:06 ` Markus Wichmann
  2022-07-11  4:48 ` Florian Weimer
  1 sibling, 0 replies; 8+ messages in thread
From: Markus Wichmann @ 2022-07-10 18:06 UTC (permalink / raw)
  To: musl

On Sun, Jul 10, 2022 at 04:32:51PM +0200, Tomasz Duda wrote:
> Hi,
>
> I'm using
> python:3.9-alpine3.14
> It looks that AF_LOCAL is not implemented.
> It would be nice if you could include it.
>
> BR,
> TD
>

It would have been nice to mention you mean getnameinfo(). I wonder if
this is even valid, since you cannot call getaddrinfo() with the node
and service gained from here and get the socket address back out. I
skimmed POSIX on these functions but could not identify if adding
AF_LOCAL is OK.

> diff --git a/src/network/getnameinfo.c b/src/network/getnameinfo.c
> index 949e181..7d3a9c3 100644
> --- a/src/network/getnameinfo.c
> +++ b/src/network/getnameinfo.c
> @@ -11,7 +11,9 @@
> #include <resolv.h>
> #include "lookup.h"
> #include "stdio_impl.h"
> +#include <sys/un.h>
> +#define MIN(a,b) ((a)<(b) ? (a) : (b))
> #define PTR_MAX (64 + sizeof ".in-addr.arpa")
> #define RR_PTR 12
> @@ -118,6 +120,29 @@ static int dns_parse_callback(void *c, int rr, const
> void *data, int len, const
> }
> +/*
> + * getnameinfo_local():
> + * Format an local address into a printable format.
> + */

What is this? Documentation comments are not present anywhere else in
the code.

> +/* ARGSUSED */

What is this? I've seen these comments before, and they are usually used
to silence some kind of linter. But again, these are not used anywhere
else in the code.

> +static int
> +getnameinfo_local(const struct sockaddr *sa, socklen_t salen,
> + char *host, socklen_t hostlen, char *serv, socklen_t servlen,
> + int flags __attribute__((unused)))

We generally don't use GCC attributes unless we can help it. In this
case there is no reason for the attribute, but neither is there for the
argument. You could just drop it.

> +{
> + const struct sockaddr_un *sun =
> + (const struct sockaddr_un *)(const void *)sa;

Why the two conversions? Wouldn't either one suffice?

> + if (salen < (socklen_t) offsetof(struct sockaddr_un, sun_path)) {
> + return EAI_FAMILY;
> + }
> + if (serv != NULL && servlen > 0)
> + serv[0] = '\0';
> + if (host && hostlen > 0)
> + strncpy(host, sun->sun_path,
> + MIN((socklen_t) sizeof(sun->sun_path) + 1, hostlen));
> + return 0;
> +}
> +

The sizeof expression is invalid; the size of the path field is
salen - offsetof(struct sockaddr_un, sun_path). The + 1 is invalid as
well (sizeof already gives you the maximum, anyway, you cannot validly
go beyond it). And the construction ignores abstract addresses (first
path byte is zero, then the name is whatever octets follow, for however
much the length given is).

Ciao,
Markus

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] AF_LOCAL
  2022-07-10 14:32 [musl] AF_LOCAL Tomasz Duda
  2022-07-10 18:06 ` Markus Wichmann
@ 2022-07-11  4:48 ` Florian Weimer
  2022-07-11  7:54   ` Tomasz Duda
  1 sibling, 1 reply; 8+ messages in thread
From: Florian Weimer @ 2022-07-11  4:48 UTC (permalink / raw)
  To: Tomasz Duda; +Cc: musl

* Tomasz Duda:

> I'm using 
> python:3.9-alpine3.14
> It looks that AF_LOCAL is not implemented.
> It would be nice if you could include it.

There are multiple incompatible implementations of AF_LOCAL for
getnameinfo.  Some put the path into the host, others put it into the
service.  I'm not sure if it's a good idea to implement it.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] AF_LOCAL
  2022-07-11  4:48 ` Florian Weimer
@ 2022-07-11  7:54   ` Tomasz Duda
  2022-07-11  7:56     ` Florian Weimer
  0 siblings, 1 reply; 8+ messages in thread
From: Tomasz Duda @ 2022-07-11  7:54 UTC (permalink / raw)
  To: Florian Weimer; +Cc: musl

[-- Attachment #1: Type: text/plain, Size: 508 bytes --]

Hi,

It seems to be implemented in other libs.

BR,
TD

On Mon, Jul 11, 2022, 13:48 Florian Weimer <fweimer@redhat.com> wrote:

> * Tomasz Duda:
>
> > I'm using
> > python:3.9-alpine3.14
> > It looks that AF_LOCAL is not implemented.
> > It would be nice if you could include it.
>
> There are multiple incompatible implementations of AF_LOCAL for
> getnameinfo.  Some put the path into the host, others put it into the
> service.  I'm not sure if it's a good idea to implement it.
>
> Thanks,
> Florian
>
>

[-- Attachment #2: Type: text/html, Size: 952 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] AF_LOCAL
  2022-07-11  7:54   ` Tomasz Duda
@ 2022-07-11  7:56     ` Florian Weimer
  2022-07-11  8:04       ` Tomasz Duda
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Weimer @ 2022-07-11  7:56 UTC (permalink / raw)
  To: Tomasz Duda; +Cc: musl

* Tomasz Duda:

> It seems to be implemented in other libs.

Yes, but not in a consistent fashion.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] AF_LOCAL
  2022-07-11  7:56     ` Florian Weimer
@ 2022-07-11  8:04       ` Tomasz Duda
  2022-07-11 15:22         ` Rich Felker
  0 siblings, 1 reply; 8+ messages in thread
From: Tomasz Duda @ 2022-07-11  8:04 UTC (permalink / raw)
  To: Florian Weimer; +Cc: musl

[-- Attachment #1: Type: text/plain, Size: 300 bytes --]

Done is better than perfect. It prevents some applications from starting so
it is pretty bad.

On Mon, Jul 11, 2022, 16:56 Florian Weimer <fweimer@redhat.com> wrote:

> * Tomasz Duda:
>
> > It seems to be implemented in other libs.
>
> Yes, but not in a consistent fashion.
>
> Thanks,
> Florian
>
>

[-- Attachment #2: Type: text/html, Size: 625 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] AF_LOCAL
  2022-07-11  8:04       ` Tomasz Duda
@ 2022-07-11 15:22         ` Rich Felker
  2022-07-11 19:53           ` Tomasz Duda
  0 siblings, 1 reply; 8+ messages in thread
From: Rich Felker @ 2022-07-11 15:22 UTC (permalink / raw)
  To: Tomasz Duda; +Cc: Florian Weimer, musl

On Mon, Jul 11, 2022 at 05:04:15PM +0900, Tomasz Duda wrote:
> Done is better than perfect. It prevents some applications from starting so
> it is pretty bad.

No, inconsistent is worse than not at all. Not-at-all forces
applications to fix the wrong assumption. Inconsistent means
applications compile and run but silently (in sense of not producing
an error at build and/or run time) do the wrong thing on some systems.
One of the main criteria for exclusion of an extension from musl is
the existence of multiple conflicting historical definitions for the
same extension.


> On Mon, Jul 11, 2022, 16:56 Florian Weimer <fweimer@redhat.com> wrote:
> 
> > * Tomasz Duda:
> >
> > > It seems to be implemented in other libs.
> >
> > Yes, but not in a consistent fashion.
> >
> > Thanks,
> > Florian
> >
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [musl] AF_LOCAL
  2022-07-11 15:22         ` Rich Felker
@ 2022-07-11 19:53           ` Tomasz Duda
  0 siblings, 0 replies; 8+ messages in thread
From: Tomasz Duda @ 2022-07-11 19:53 UTC (permalink / raw)
  To: Rich Felker; +Cc: Florian Weimer, musl

[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]

I love alpine since it is small distribution which have a lot of packages.
The problem is that I ended up patching libc during building docker image.
I guess that I'm not only one who meet this problem. For most users it
would be deal breaker.
https://github.com/pikvm/kvmd/pull/101/commits/5698bf29d5948e79bffd9a7bebeec77e39d8f18f#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557



pon., 11 lip 2022 o 17:22 Rich Felker <dalias@libc.org> napisał(a):

> On Mon, Jul 11, 2022 at 05:04:15PM +0900, Tomasz Duda wrote:
> > Done is better than perfect. It prevents some applications from starting
> so
> > it is pretty bad.
>
> No, inconsistent is worse than not at all. Not-at-all forces
> applications to fix the wrong assumption. Inconsistent means
> applications compile and run but silently (in sense of not producing
> an error at build and/or run time) do the wrong thing on some systems.
> One of the main criteria for exclusion of an extension from musl is
> the existence of multiple conflicting historical definitions for the
> same extension.
>
>
> > On Mon, Jul 11, 2022, 16:56 Florian Weimer <fweimer@redhat.com> wrote:
> >
> > > * Tomasz Duda:
> > >
> > > > It seems to be implemented in other libs.
> > >
> > > Yes, but not in a consistent fashion.
> > >
> > > Thanks,
> > > Florian
> > >
> > >
>

[-- Attachment #2: Type: text/html, Size: 2069 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-07-11 19:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-10 14:32 [musl] AF_LOCAL Tomasz Duda
2022-07-10 18:06 ` Markus Wichmann
2022-07-11  4:48 ` Florian Weimer
2022-07-11  7:54   ` Tomasz Duda
2022-07-11  7:56     ` Florian Weimer
2022-07-11  8:04       ` Tomasz Duda
2022-07-11 15:22         ` Rich Felker
2022-07-11 19:53           ` Tomasz Duda

Code repositories for project(s) associated with this 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).