On 15 June 2017 at 04:16, Michel Dänzer <michel at daenzer.net> wrote: > On 14/06/17 08:34 PM, Emil Velikov wrote: >> On 13 June 2017 at 10:45, Michel Dänzer <michel at daenzer.net> wrote: >>> From: Xiaojie Yuan <Xiaojie.Yuan at amd.com> >>> >>> v2: fix an off by one error and leading white spaces >>> v3: use thread safe strtok_r(); initialize len before calling getline(); >>> change printf() to drmMsg(); add initial amdgpu.ids >>> v4: integrate some recent internal changes, including format changes >>> v5: fix line number for empty/commented lines; realloc to save memory; >>> indentation changes >>> v6: remove a line error >>> v7: [Michel Dänzer] >>> * Move amdgpu.ids to new data directory >>> * Remove placeholder entries from amdgpu.ids >>> * Set libdrmdatadir variable in configure.ac instead of Makefile.am >>> [Emil Velikov] >>> * Use isblank() instead of open-coding it [Emil Velikov] >>> * Don't leak asic_id_table memory if realloc fails [Emil Velikov] >>> * Check and bump table_max_size at the beginning of the while loop [Emil >>> Velikov] >>> * Initialize table_max_size to the number of entries in data/amdgpu.ids >> Thank you for addressing some of my suggestions. >> Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com> > > Thanks! Pushed. > > >> Personally I would not have bothered with the table_max_size thing > > It seemed silly to reallocate the memory in the default case where the > amdgpu.ids file from this repository is used. :) > Agreed. Yet the single, "reduce memory consumption" realloc seems to diminish amongst the ~150 [unneeded] strdup/free, in parse_one_line </tease> >> or the separate Makefile. > > You mean data/Makefile.am? What would you have done instead? > One can fold the two lines within the top makefile (see below) since I'm lazy to complete the "use non-recursive makefiles" [1] branch. -Emil [1] https://github.com/evelikov/libdrm/commits/hello-world --- a/Makefile.am +++ b/Makefile.am @@ -43,6 +43,9 @@ AM_DISTCHECK_CONFIGURE_FLAGS = \ --enable-manpages \ --enable-valgrind +libdrmdatadir = @libdrmdatadir@ +dist_libdrmdata_DATA = data/amdgpu.ids + pkgconfigdir = @pkgconfigdir@ pkgconfig_DATA = libdrm.pc
