Re: [flashrom] [PATCH 1/5] layout: change return type and name of find_next_included_romentry

[Carl-Daniel Hailfinger]

Am 24.12.2011 01:35 schrieb Stefan Tauner:
> rename from find_next_included_romentry to get_next_included_romentry
> return a pointer to a rom_entry instead of just its index
>
> Signed-off-by: Stefan Tauner

The changelog only describes what you're doing, but not the reason why.
If you add a short statement about the reason (readability improvement,
whatever) to the changelog, and if you change
if (entry == NULL) {
to
if  (!entry) {
this is
Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel

[PATCH 1/5] layout: change return type and name of find_next_included_romentry

[Stefan Tauner]

On Sun, 25 Dec 2011 01:08:34 +0100
Carl-Daniel Hailfinger  wrote:
> Am 24.12.2011 01:35 schrieb Stefan Tauner:
> > rename from find_next_included_romentry to get_next_included_romentry
> > return a pointer to a rom_entry instead of just its index
> >
> > Signed-off-by: Stefan Tauner
>
> The changelog only describes what you're doing, but not the reason why.
> If you add a short statement about the reason (readability improvement,
> whatever) to the changelog,

oh that's a hard task.
first of all because i dont remember why i did it back then. secondly
because there is no obvious reason to change it. the index allows
accessing more information than a pointer to just a single object/entry.
the benefit as i see it is - as you said - readability. but it is
also way more natural to dereference the pointer to an object and then
operate on it than accessing it by retrieving it manually from an
array. the former hides the retrieving process so that the entries
could be stored completely differently so it is also an improvement of
segregation.

ha, now it isnt that hard anymore :)
> and if you change
> if (entry == NULL) {
> to
> if  (!entry) {

i really hate, when c programmers do that. imho it's less readable
without any worthy benefits, especially if used in single-term ifs.
changed anyway.
> this is
> Acked-by: Carl-Daniel Hailfinger

thanks!
will commit it in r1481 if no one beats me to it ;)