2024年5月14日(火) 12:32 Matheus Afonso Martins Moreira <math...@matheusmoreira.com>:
> > The patches touch the interface of many functions
>
> I added one external function: find_in_path_var.
> The other changes are all static and local.
>
> > It seems essentially equivalent to just calling
> > find_in_path (list->word->word, path_value ("BASH_SOURCE_PATH", 1),
> > FS_READABLE);
>
> It is. I'm just very averse to chaining functions like that in C
> since you can't write f(g(x)) without worrying about freeing
> the value returned by g.
If you are talking about the current specific case of path_value, as
Chet has replied, path_value doesn't return a newly allocated block.
It just returns a pointer to an existing variable cell owned and
managed by the global variable context whose root is static. It
doesn't return or move an ownership of the pointer.
If you are talking about just the coding style in general, it seems
like just your personal style. In addition, the patch just
encapsulates it in another function h(x) := f(g(x)) (as done in
`_find_user_command_internal()'), and the logical structure is the
same. One could argue that `_find_user_command_internal()' is using a
separate form `var = g(x); return f(var);', but then one can also use
the separate form at the caller's side. Whether to use the combined
form or the separate form seems to be orthogonal to whether we do it
inline or in a new function.
> > The cases where path_value() returns NULL or an empty string would be
> > anyway handled by find_in_path()
>
> With find_in_path_var there's no need to even consider that.
Yes, but even without `find_in_path_var', there's no need to consider
that because it is handled elsewhere.
> > I doubt it's worth adding those changes in
> > `findcmd.c'. So are the changes in [PATCH v2 2/8].
>
> You don't think they improve the code?
I don't think so. There are always pros and cons, and the choice would
depend on many factors including the maintenance cost and the
preference of the maintainer. The inter-modular interface should be
minimized to reduce the coupling unless the module is very stable and
immutable. You might think preparing as many "buttons" as possible
would be a good interface, but any additional interface becomes a
constraint or a boundary condition to the module and adds a
maintenance cost for future changes. The final choice of how the code
is maintained would be determined by the maintainer; it is similar to
the coding style. If one observes the Bash codebase, one notices that
it is not maintained in your style.
