On 11/07, Jakub Kicinski wrote:
> On Wed, 7 Nov 2018 15:00:21 -0800, Stanislav Fomichev wrote:
> > > > +err_unpin_programs:
> > > > + bpf_object__for_each_program(prog, obj) {
> > > > + char buf[PATH_MAX];
> > > > + int len;
> > > > +
> > > > + len = snprintf(buf, PATH_MAX, "%s/%s", path,
> > > > + prog->section_name);
> > > > + if (len < 0)
> > > > + continue;
> > > > + else if (len >= PATH_MAX)
> > > > + continue;
> > > > +
> > > > + unlink(buf);
> > >
> > > I think that's no bueno, if pin failed because the file already exists
> > > you'll now remove that already existing file.
> >
> > How about we check beforehand and bail early if we are going to
> > overwrite something?
>
> Possible, although the most common way to handle situation like this in
> the kernel is to "continue the iteration in reverse" over the list.
> I.e. walk the list back. I think the objects are on a double linked
> list. You may need to add the appropriate foreach macro and helper..
That sounds more complicated than just ensuring that the top directory
for the pins doesn't exist and then rm -rf it on failure.
I'm thinking about copy-pasting rm_rf from perf
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/util.c#n119).
Thoughts?
Btw, current patch won't work because of those /0 added by bpf_program__pin.
