Messages in this thread | | | Date | Mon, 18 May 2015 17:37:00 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [RFC PATCH v3 21/37] bpf tools: Create eBPF maps defined in an object file |
| |
Em Mon, May 18, 2015 at 11:48:57AM -0700, Alexei Starovoitov escreveu: > On 5/17/15 3:56 AM, Wang Nan wrote: > >This patch creates maps based on 'map' section in object file using > >bpf_create_map(), and store the fds into an array in > >'struct bpf_object'. Since the byte order of the object may differ > >from the host, swap map definition before processing. > > > >This is the first patch in 'loading' phase. Previous patches parse ELF > >object file and create needed data structure, but doesnnt play with > >kernel. They belong to 'opening' phase. > > > >Signed-off-by: Wang Nan <wangnan0@huawei.com> > ... > >+ for (j = 0; j < i; j++) { > >+ close(obj->maps_fds[j]); > >+ obj->maps_fds[j] = -1; > > this line is unnecessary, since you're freeing the whole array > at the line below:
I guess this is kinda of a zfree() that he wants to achieve, i.e. if a bug creeps in that makes some code use the deleted maps_fds, he rather wants it to access a -1 fd than an some reused fd, no?
> >+ } > >+ free(obj->maps_fds);
> >+ obj->maps_fds = NULL; > > ... > > > void bpf_close_object(struct bpf_object *obj) > > { > > if (!obj) > > return; > > > > bpf_obj_clear_elf(obj); > >+ bpf_unload_object(obj); > > just realized that this API keeps elf file open for the whole > life time. I think that should be split up. > bpf_open_object() can do elf parsing, creation of maps and > bpf loading. > bpf_close_object() can unload programs, maps. That's fine, > but closing of elf file and freeing memory associated > with parsing sections should be a separate call.
| |