I've updated the man page for the new version. Is this what you had in mind?

thank you very much for your feedback.

I've added a few comments and will fix the other issues.

this was taken from dladm_door_call() in usr/src/lib/libdladm/common/libdlmgmt.c

I'll fix this here, should I also update the original source?

If you wish to, I didn't want to mark this as issue, just as hint.

From looking at dladm_walk_macaddr() in usr/src/lib/libdladm/common/libdllink.c

It's strange that we don't have a define for this.

that looks indeed much better. I wasn't sure if it was safe to call close() like that. I guess I'll need to make sure the sockets are initialized to something invalid like -1?

Yeah, hopefully others won't find this bad/ugly :-) and even if libsocket is not linted, let's cast the return values of close() to (void), showing that we really don't care about its return value.

Yuri, why for(;;) is better  than while (1) ?

Even if we don't lint libsocket, just to be consistent with other parts of the code where lint would whine about using constant condition. And a lot of style guides I did check (from time to time) do say "Forever loops are done with for's, not while's.".

Never heard about that. It looks as overhead styles :) Consisetency is good reason. Thanks.

It does not matter if we check with v4 or v6 because the information should be the same.

The interface index is useful to match an ip address to a mac address.

My point is that if we can't rely on this information to be available always, it's not that useful.

In the case where we have an ip address and want to match it to a mac adress it is available, so it is there when we need it.
But I've looked a bit closer at this again and indeed the metric is NOT guaranteed to be the same. So I'll adjust it to not provide it for AF_LINK entries.

As there is already rbuf I've changed the return values around a bit. I hope this makes things cleaner. Let me know if this is what you had in mind and if I can improve things further.

Sure, this works as a way to do this, thanks!

I ran lint manually and got:

warning: function returns value which is always ignored: munmap (E_FUNC_RET_ALWAYS_IGNOR2)
Not sure how I can handle that better or if it should be ignored. Happy to drop the cast if that is the preferred style.

In asking this, I more so meant why are we ignoring the return value. If munmap returns EINVAL what does that mean? Does that mean there was a programmer error from us or that something is really wrong with the system?

quote from door_call(3C):
If the results of a door invocation exceed the size of the buffer
specified by rsize, the system automatically allocates  a new buffer in
the caller's address space  and updates the rbuf and rsize members to
reflect this location. In this case, the caller is  responsible for
reclaiming this area using munmap(rbuf, rsize) when the buffer is no
longer required.  See munmap(2).


So if something is wrong the data we have came from the door_call().

I don't think any input from us should cause door_call() to return a buffer that we'd have to cleanup but can't.
I've looked through grok and nearly all munmap calls have their return values casted away.

There are a few in testcases that have asserts and a few others that actually do some handling like printing a message.

OK, fair enough. Whenver I see things that may return errors cast to void, I always want to ask. I think you're right that hopefully door_call will put something right there. If it did fail, I'm not sure if we'd want to drive on, but the much more damaging case of unmapping the wrong thing we can't tell anyways so this is probably fine.

I've changed the return paths a bit now to explicitly return -1 or 0, but could still make it boolean_t if you think that is better.

The 0 and -1/errno pattern is fine. Though I made a note about the errno consistency bit on those. Do you want to preserve the return value from dlmgmtd as the errno in those cases? Though I know it's just discarded anyways.

The value in lr_err can actually be used directly used as errno, so this was easy. The latest version should address this.

I've changed the return paths a bit now to explicitly return -1 or 0, but could still make it boolean_t if you think that is better.

See other notes.

I've changed it to strlcpy() and also moved it a bit closer to the actual ioctl call.
Can you explain a bit more why we should not assume that they're the same?

I use it to fill in the ip interface index, but during the review process this code does seem to cause a bit of confusion.

While I understand that the mapping between the links and ip is a bit odd I still think the data would be useful.

So I wonder how I could structure that better or if my understanding is just too wrong and that part of the code should be dropped.

In this case I left out the operative word size. I was just saying that we didn't want to assume that the dlmgmtd size (in the door call) would always be the same as the buffer size.
Strictly speaking the IP interface on top of a datalink can have a different from the datalink from the kernel's perspective, but that's impossible to do with ifconfig / ipadm, etc. so I'm not sure if that case matters.

ah ok, checking for overflow is now done.

This is not much of a preference. The returned index will be the same if both IPv4 and IPv6 are on the same link. We just have to make sure we try both in case it is IPv4-only or IPv6-only.

Ah, gotcha. I wasn't sure if they had the same index or not. Thanks!

We can use lr_err for this, done in the latest version.

This was a leftover that was missed from an even earlier version which used ioctls and ip instead of doors and datalinks.

For links those ip flags don't make much sense (they are per ip address, not per link), so they should be zero.

Sorry, I mean public, of course.

I'm not sure.
I've tried finding a definite answer but only got more uncertain.
If this should be public, should it have two underscores as a start prefix? So even it is public it is marked as a internal thing? Where can I find rules/standards about this?
If it should not be public, what happens to software that is build now and links against it? It will always need it to be there, right?

Since we redirect people trying to use the public getifaddrs(3socket) to this symbol, it seems like this symbol needs to be at least as stable as getifaddrs(3socket),