[Toybox] [PATCH] tail: implement -F (and its companion -s).

Rob Landley rob at landley.net
Thu Jul 1 09:47:10 PDT 2021


On 6/30/21 6:57 PM, enh via Toybox wrote:
> (Based on someone else's patch.)
> 
> Implementing -F with inotify is a lot more work (including more
> portability shims for macOS), so this is a simpler polling
> implementation.
> 
> Also fix my earlier mistake where xnotify_add() wasn't actually an 'x'
> function that exits on failure.

Applied, but:

struct follow_file {
  char *path;
  int fd;
  dev_t dev;
  ino_t ino;
};

GLOBALS(
  long n, c, s;
  int file_no, last_fd;
  struct xnotify *not;
  struct follow_file *F;
)

Means that generated/globals.h has a pointer to an incomplete type, which the
compiler allows (the union member is a pointer and it knows the size of that
regardless what it points to) but makes me uncomfortable.

What you're doing works, but possibly not for the _reason_ you think it does?
Specifically, this patch has a structure defined before the GLOBALS() block and
another defined after, and they might as well both be defined after because the
one defined before GLOBALS() doesn't wind up in the header everything else
includes, nor is it used to calculate the size of the union. So splitting them
serves no purpose but _looks_ like it does.

In toysh I'm doing the equivalent of:

GLOBALS(
  long n, c, s;
  int file_no, last_fd;
  struct xnotify *not;
  struct tail_follow_file {
    char *path;
    int fd;
    dev_t dev;
    ino_t ino;
  } *F;
)

Which is also a little awkward (nested structure definitions wind up in a global
namespace, type prefixed with command name to avoid collisions in said global
namespace...), but doing that allows me to embed instances of instructures and
not just pointers to them. Elsewhere I've had the global one be a void * and
typecast it on use, which is differntly awkward. This third way isn't
necessarily MORE awkward, but I dislike having 3 ways to do the same thing in
play in the same codebase.

This is mostly a "code style" thing, I need to work out the preferred style for
dealing with the context switch, and document the issue. (The compiler is
forgiving, but I don't want to confuse people reading the code...)

Rob



More information about the Toybox mailing list