Blank line after nonexistent local variables.

This function can be static.

This indentation doesn't look like it follows style(9)

No line break

Newline before function name.

This can be static.

Add context to the diff. Even without the context, I am sure that you put the symbols into the wrong namespace.

Do not put initialization into local vars declaration.

That said, error var is pointless, just do return (kern_getfhat());.

There must be a blank line after '{' if there is no locals.

Continuation line should use +4 spaces.

if (path != NULL)

This comment is pointless. Either create a helper function used by both, or remove it.

Should I add a FBSD_1.6 section then?

nevermind, I just realized https://wiki.freebsd.org/AddingSyscalls says to put them in the most recent namespace (FBSD_1.5)

No, FBSD_1.5 is wrong again. New 13-CURRENT symbols must go into FBSD_1.6.

Blank line is needed after '{' since there are no local vars.

Wrong indent. Please review all the code for style issues that were already pointed out.

Put the binary op designator before the split.

flags & AT_BENEATH is not boolean, use (flags & AT_BENEATH) != 0. Same for AT_SYMLINK_NOFOLLOW.

LR_RETRY is allowed to return doomed vnode. You cannot access v_mount or call any VOP method on it.

That said, I find having the path == NULL case allowed for userspace somewhat strange. We do not allow NULL path for open(2), and we have getfh(2). I understand that this case allows to implement sys_gethf(), I am more about checking for path == NULL in getfhat().

Again blank line after '{'.

Same question there, IMO it is worth introducing the helper and avoid copy/paste.

So I actually mean that duplication of the code is wrong, and I really do not see why do we need two copies. kern_linkat() after the first namei() is indeed exact copy, so why not add the helper ?

I agree the path = NULL case shouldn't be allowed for getfh, so I moved the logic into sys_getfhat.

I've added a helper function, the code was not _exactly_ the same so I had to change the tag/goto into a do while/EAGAIN, also has to add a locked variable. If these changes are ok by you I think it's much cleaner now.

getfh is not enabled in cap mode, why do you enable getfhat ?

Wrong indent for continuation line.

There too.

Blank line is not needed.

Blank line is not needed.

See the comment about vn_XXX namespace.

Wrong indentation for the continuation line

This call locks vnodes internally. Your use of vn_linkat() while the vp is locked can cause lock recursion at least. Unfortunately WITNESS cannot detect the issues because vnode locks have same names.

vn_XXX namespace is for functions in vfs_vnops.c. It is somewhat strange to use it for vfs_syscalls.c

Again indent.

I recomment to put an assert that vp is locked there.

It seems that you moved vput(vp) out of the helper's code to both callers. Is this done to make the vn_readlink() interface consistent (locked vnode in/locked vnode out) ?

No need to put {} around single line.

Stray blank line, more such lines below.

Remove all blank lines from the function starting with this one, they are not needed/allowed by style.

s/link/fhlink/ ?

I'd use "points to an invalid address." here.

I wasn't sure about the naming, what would you recommend?

yes, I thought it would be cleaner to let the caller of vn_readlink manage the ressource

It seems that mac_vnode_check_readlink() already performs an ASSERT_VOP_LOCKED() within.

kern_readlink_vp() for instance

mac_ stuff is conditional. Adding asserts at the start of the functions document their pre-conditions. We do not rely on asserts in the VOP_READLINK() wrapper, for similar reasons.

we perform a vput() before the first EAGAIN, the second EAGAIN will never be reached because the locked case will match the previous else is. so all shold be fine? maybe i could add a comment

I do not see how can you claim that there is no longer the lock order/lock recursion issue. namei() locks the ni_vp vnode internally, and if LOCK_LEAF flag is not provided, it unlocks the vnode before returning. You simply cannot call namei() while any vnode is locked, it is very basic rule of our VFS.

Why exclusive lock ?

linkat doesn't LOCK_LEAF on the source node either so I've just added a VOP_UNLOCK as I haven't found a way to get VFS_FHTOVP to unlock before returning, the documenation is outdated and doesn't even show the flags argument to the macro.

you're right, readlinkat(2) does LOCKSHARED . behaviour should be the same with`fhreadlinkat`

This looks incomplete.

Missing a period.

I am sure that UCB has nothing to do with the new file. Please update copyright with the proper reference (to your personally or to Gandi). Same for fhreadlink.

BTW, preferred license for the new files is two clause BSD, see share/examples/etc/bsd-style-copyright. Please consider using it.

You cannot remove other people license. You can add your copyright notice since you added substantial new content, but since the the original content is there, you must not touch the UCB statement.

Look at other files how multiple copyright notices are handled.