HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com> writes: > From: "Eric W. Biederman" <ebiederm at xmission.com> > Subject: Re: [PATCH v3 01/21] vmcore: reference e_phoff member explicitly to get position of program header table > Date: Tue, 19 Mar 2013 14:44:16 -0700 > >> HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com> writes: >> >>> Currently, the code assumes that position of program header table is >>> next to ELF header. But future change can break the assumption on >>> kexec-tools and the 1st kernel. To avoid worst case, reference e_phoff >>> member explicitly to get position of program header table in >>> file-offset. >> >> In principle this looks good. However when I read this it looks like >> you are going a little too far. >> >> You are changing not only the reading of the supplied headers, but >> you are changing the generation of the new new headers that describe >> the data provided by /proc/vmcore. >> >> I get lost in following this after you mangle merge_note_headers. >> >> In principle removing silly assumptions seems reasonable, but I think >> it is completely orthogonal to the task of maping vmcore mmapable. >> >> I think it is fine to claim that the assumptions made here in vmcore are >> part of the kexec on panic ABI at this point, which would generally make >> this change unnecessary. > > This was suggested by Vivek. He prefers generic one. > > Vivek, do you agree to this? Or is it better to re-post this and other > clean-up patches as another one separately to this patch set? My deep problem with this patch was that you were changing how we think about the generated headers, and that just didn't seem to make any sense, and certainly it seems to be orthogonal from the patch itself. For headers that we generate it is perfectly fine to make all kinds of assumptions. Eric