Think of a similar scope of change to a large codebase you’re familiar with, for frame of reference.

  • The Octonaut@mander.xyz
    link
    fedilink
    arrow-up
    1
    ·
    9 months ago

    Yeah I get that but you dig deeper and that implantation was just throwing an error that needs to be handled elsewhere. The “real” code is what is handling that error.

    But then we’re back to act acknowledging a meaningful point of having commits that do one thing and do it well and understandably, and I’m back to appreciating the difference between the kernel and our app.

    • mozz@mbin.grits.devOP
      link
      fedilink
      arrow-up
      1
      ·
      9 months ago

      There is no code somewhere else that needed to be written somewhere else to handle the error. It just started working because those lines were added. That’s what I keep saying. That is the nature of the good design I am pleased by to give the example.

      I’m such a stubborn person that I literally had a look in namei.c to see what happens from that return, to make sure. I was fairly confident that that’s how it works, just because it’s rare in kernel development to add a feature without the means to trigger it and then add the means to trigger it in a separate patch (why would it not just all go in at once?), but I had a look and here’s what I saw:

      • get_link() returns ELOOP
      • In one case, vfs_readlink() returns an error straight to the user after going through the VFS layer
      • In the more complex case where we were actually trying to follow it to do some other operation, pick_link() sees the error and returns an error
      • Which is what gets returned from step_into()
      • step_into() gets called from a bunch of places, but I looked at them briefly and quickly searched for ELOOP and NOSYMFOLLOW and convinced myself pretty well that there isn’t anywhere that handles the error any differently than it would any other we-can’t-do-this error, and ultimately it’s going to just return the failure to the user. Because why would it do anything different? It looks to me although I’m not sure like step_into() up to walk_component() up to link_path_walk() up to path_lookupat() or path_openat() is sort of the main path you’d get to from userspace via do_file_open() or similar functions, and they all plan to just return the refusal to follow the symlink back to userspace.

      Where are you getting your assertion that obviously it’s already implemented somewhere else separate from this patch?