[PATCH 2/2] NAND: nandsim sector-wise allocation

Vijay Kumar vijaykumar at bravegnu.org
Sun Oct 8 01:50:08 EDT 2006


    Artem> I wonder, why "#if defined(kaka)" is better then "#ifdef
    Artem> kaka" ? What for is this change :-) ?

A kind of symmetry, since there is no #elifdef! :-) Anyway, the
#ifdefs are no longer required.

    >> +union ns_mem_t {
    >> +	u_char *byte;
    >> +	uint16_t *word;
    >> +};

    Artem> Minor, but 'union ns_mem' is a better name. We usually use
    Artem> "_t" for typedefs, and we use typedefs very rarely and only
    Artem> if there is a good reason.

Point taken.

    >> +static int
    >> +alloc_map_device(struct nandsim *ns)

    Artem> Again minor. I understand that everybody has its own style,
    Artem> and I appreciate this. But when one fixes an existing code,
    Artem> it makes sense to follow the same style. So I offer to use
    Artem> definitions like

Actually, the nandsim code uses this style! I used the same style to
be consistent with the nandsim code. I won't be fixing this in my
updated patch. Probably we could have a separate patch that fixes all
function definitions.

    Artem> Again a nit. We usually don't use brackets in case of
    Artem> one-line code - just a convention.

Point taken.

Thanks for the review Artem. Will send an updated patch shortly.

Regards,
Vijay

-- 
Free the Code, Free the User.
Website: http://www.bravegnu.org





More information about the linux-mtd mailing list