> On Nov. 14, 2015, 7:18 p.m., Nilay Vaish wrote: > > src/base/loader/object_file.cc, line 144 > > <http://reviews.gem5.org/r/3131/diff/2/?file=51644#file51644line144> > > > > Does mmap complain with nullptr?
No, it can be nullptr. > On Nov. 14, 2015, 7:18 p.m., Nilay Vaish wrote: > > src/base/loader/object_file.cc, line 177 > > <http://reviews.gem5.org/r/3131/diff/2/?file=51644#file51644line177> > > > > Does this check fail if doGzipLoad() returns nullptr? Ah, this is a good point. The return value from doGzipLoad should be MAP_FAILED in the two places it returns nullptr. On Linux MAP_FAILED is all ones. > On Nov. 14, 2015, 7:18 p.m., Nilay Vaish wrote: > > src/base/loader/object_file.cc, lines 165-176 > > <http://reviews.gem5.org/r/3131/diff/2/?file=51644#file51644line165> > > > > I still think this code should be common to both cases. Your function > > doGzipLoad() should return the fd to the tmp file you create. because it's opened with tmpfile(), which is a libc function returning FILE*, it should be closed with fclose. We can't return the fd after fclose()'ing it because the fd will be closed. If we return the fd without fclose()'ing it, then gem5 will breach the contract with libc and leak memory; if we return the FILE* then the code will be messier due to even more control divergence. - Curtis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3131/#review7586 ----------------------------------------------------------- On Nov. 14, 2015, 12:01 a.m., Curtis Dunham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3131/ > ----------------------------------------------------------- > > (Updated Nov. 14, 2015, 12:01 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Automatically detect and decompress gzip'd content in the object loader. > > > Diffs > ----- > > src/base/loader/object_file.cc 2375b33bddc61ea484b3bd194ba02f7889095624 > > Diff: http://reviews.gem5.org/r/3131/diff/ > > > Testing > ------- > > > Thanks, > > Curtis Dunham > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
