loader: update symlink support in zfs reader
ClosedPublic

Authored by tsoome on Feb 21 2017, 8:27 PM.

Details

Summary

As the current zfs file system is providing symlink via system attributes, need to update the code accordingly.

Test Plan

Tested by browsing and reading different symlinks and by using names like /usr/bin/../lib, bin/.., etc/., etc/./inet, ./bin

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
tsoome retitled this revision from to loader: update symlink support in zfs reader.Feb 21 2017, 8:27 PM
tsoome updated this object.
tsoome edited the test plan for this revision. (Show Details)
tsoome added reviewers: allanjude, imp.
avg accepted this revision.Feb 22 2017, 8:20 AM

LGTM

This revision is now accepted and ready to land.Feb 22 2017, 8:20 AM
smh requested changes to this revision.Feb 22 2017, 9:31 AM
smh added inline comments.
sys/boot/zfs/zfsimpl.c
2273 ↗(On Diff #25494)

All of the above are only used in == DMU_OT_SA case so could be moved into that block with char *p;

2294 ↗(On Diff #25494)

Does the boot code make use of SET_ERROR, I'm guessing not?

2382 ↗(On Diff #25494)

Missing space

This revision now requires changes to proceed.Feb 22 2017, 9:31 AM
tsoome marked 3 inline comments as done.Feb 22 2017, 12:21 PM
tsoome added inline comments.
sys/boot/zfs/zfsimpl.c
2294 ↗(On Diff #25494)

no, it does not.

tsoome updated this revision to Diff 25520.Feb 22 2017, 12:22 PM
tsoome marked an inline comment as done.

Fixes suggested by smh.

smh added inline comments.Feb 22 2017, 12:31 PM
sys/boot/zfs/zfsimpl.c
2282 ↗(On Diff #25520)

Just spotted that if you invert the condition you get an arguably cleaner flow e.g.

 else {
    if ((dn->dn_flags & DNODE_FLAG_SPILL_BLKPTR) == 0)
        return (EIO);

    blkptr_t *bp = &dn->dn_spill;

    size = BP_GET_LSIZE(bp);
    ...
}
tsoome updated this revision to Diff 25521.Feb 22 2017, 12:42 PM

svn update; and clean up the inverted if statement.

tsoome marked an inline comment as done.Feb 22 2017, 12:42 PM
smh accepted this revision.Feb 22 2017, 1:42 PM

LGTM

This revision is now accepted and ready to land.Feb 22 2017, 1:42 PM
allanjude accepted this revision.Feb 22 2017, 2:57 PM

Approved By: allanjude

This revision was automatically updated to reflect the committed changes.