"Magnus Damm" <[EMAIL PROTECTED]> writes:
> On 11/22/06, Vivek Goyal <[EMAIL PROTECTED]> wrote:
>> On Tue, Nov 21, 2006 at 03:25:39PM +0900, Magnus Damm wrote:
>> [..]
>> > >> +int kexec_iomem_single(char *str, uint64_t *start, uint64_t *end)
>> > >> +{
>> > >
>> > >Would "parse_iomem_single" be a better name for the function?
>> >
>> > I usually name my functions with the filename as prefix - hence the
>> > "kexec_iomem". But I don't mind changing the name - my main reason
>> > behind this is to reduce code redundancy so I'm willing to give in on
>> > almost everything except on code removal. =)
>> >
>> > >> + struct memory_range range;
>> > >> + int ret;
>> > >> +
>> > >> + memset(&range, 0, sizeof(range));
>> > >> +
>> > >> + ret = kexec_iomem_for_each_line(str, kexec_iomem_single_callback,
>> > >> + &range);
>> > >> +
>> > >> + if (ret == 1) {
>> > >
>> > >I think everywhere now we are using the convention that return code 0 is
>> > >success otherwise its failure. Can we stick to that convention.
>> >
>> > Well, all functions except kexec_iomem_for_each_line() use that
>> > convention. The kexec_iomem_for_each_line() code returns the number of
>> > matched lines, and the Xen code does rely on that functionality today.
>> >
>> > I can of course change the code but that will result in some code
>> > duplication. =) Maybe it is ok as is today now when you know the
>> > reason. Is adding a comment to describe the function and return value
>> > good enough?
>>
>> If it is not too much of code, can't we return the number of lines matched
>> in a parameter and keep the return value to continue to function as error
>> code. IMHO, it is good to be consistent with rest of the functions.
>
> Sure, either way is ok with me, it does not matter. But what does
> matter to me is to get the code merged, so maybe we can add this as a
> separate fix on top of my code?
If you start playing fancy games with /proc/iomem parsing I suggest
some one write a simple general parser that stores the parsed file
in an array. With elements something like:
unsigned long long start;
unsigned long long end;
char *type;
Then just have all of the people who need to get creative walk the
stupid array.
That doesn't need callback or any other complications. Just simple
code doing simple things.
Eric
_______________________________________________
fastboot mailing list
fastboot@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/fastboot
