On Sat, Mar 14, 2009 at 12:01:13AM +0100, phcoder wrote:
> Robert Millan wrote:
>> On Fri, Mar 13, 2009 at 09:41:42PM +0100, phcoder wrote:
>>> Robert Millan wrote:
>>>> On Wed, Mar 11, 2009 at 10:21:41PM +0100, phcoder wrote:
>>>>> Robert Millan wrote:
>>>>>> On Mon, Mar 02, 2009 at 01:35:06AM +0100, phcoder wrote:
>>>>>>> + * include/grub/elf.h: added missing attributes
>>>>>> This should be a bit more descriptive.
>>>>>>
>>>>>>> for (i = 0; i < ehdr->e_phnum; i++)
>>>>>>> if (phdr(i)->p_type == PT_LOAD && phdr(i)->p_filesz != 0)
>>>>>>> {
>>>>>>> - if (phdr(i)->p_paddr < phdr(lowest_segment)->p_paddr)
>>>>>>> + if (lowest_segment == -1 + || phdr(i)->p_paddr <
>>>>>>> phdr(lowest_segment)->p_paddr)
>>>>>>> lowest_segment = i;
>>>>>>> - if (phdr(i)->p_paddr > phdr(highest_segment)->p_paddr)
>>>>>>> + if (highest_segment == -1
>>>>>>> + || phdr(i)->p_paddr > phdr(highest_segment)->p_paddr)
>>>>>>> highest_segment = i;
>>>>>>> }
>>>>>> Why?
>>>>> Because if first segment doesn't have the PT_LOAD attribute set
>>>>> then it should be considered in this comparison
>>>> But you didn't remove the PT_LOAD check. And in the routine below that
>>>> does the actual segment load, we still check for PT_LOAD. Those should be
>>>> consistent, right?
>>>>
>>> No I expressed myself badly. Original code assumed that first segment
>>> has PT_LOAD always set (lowest_segment is 0 initally). I removed
>>> this assumption
>>
>> Why do we care about non-PT_LOAD segments?
>
> We don't but without this fix non-PT_LOAD segment 1 wasn't correctly ignored
Oh, of course... This part of the patch is fine. Perhaps a comment would be
a good idea, so we don't forget.
--
Robert Millan
The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
how) you may access your data; but nobody's threatening your freedom: we
still allow you to remove your data and not access it at all."
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
