On Tue, Mar 15, 2022 at 10:28 AM Delyan Kratunov <delyank@xxxxxx> wrote: > > On Mon, 2022-03-14 at 16:13 -0700, Delyan Kratunov wrote: > > Thanks, Andrii! The nits/refactors are straightforward but have some questions > > below: > > > > On Fri, 2022-03-11 at 15:27 -0800, Andrii Nakryiko wrote: > > > > + > > > > + /* sanitize variable name, e.g., for static vars inside > > > > + * a function, it's name is '<function name>.<variable name>', > > > > + * which we'll turn into a '<function name>_<variable name>'. > > > > + */ > > > > + sanitize_identifier(var_ident + 1); > > > > > > btw, I think we don't need sanitization anymore. We needed it for > > > static variables (they would be of the form <func_name>.<var_name> for > > > static variables inside the functions), but now it's just unnecessary > > > complication > > > > How would we handle static variables inside functions in libraries then? > > Ah, just realized 31332ccb7562 (bpftool: Stop emitting static variables in BPF > skeleton) stopped that. I'll remove the sanitization then. > yep > [...] > > > > > we don't know the name of the final object, why would we allow to set > > > any object name at all? > > > > We don't really care about the final object name but we do need an object name > > for the subskeleton. The subskeleton type name, header guard etc all use it. > > We can say that it's always taken from the file name, but giving the user the > > option to override it feels right, given the parallel with skeletons (and what > > would we do if the file name is a pipe from a subshell invocation?). > > In particular, with the current test setup, if we don't use the name param, we'd > end up inferring names like test_subskeleton_lib_linked3 from the filename. The > tests case is a bit contrived and we can work around it but there may be other > similar situations out there. > yep > > -- Delyan
