This is much better now, but I still think it'll be even better if we can have vpd_gets and vpd_getbool in same usage for both RAM stage and ROM stage. They can then live in a shared (maybe vpd_common) place.

In fact, from your current implementation I wonder if it'll make more sense to have

 vpd.c // common utils, which based on a function like vpd_load_blob(type)
 vpd_cbmem.c // fetch and build cache in CBMEM, providing vpd_load_blob from CBMEM
 vpd_romstage.c, or vpd_nocache.c // providing vpd_load_blob directly

View Change

3 comments:

• File src/drivers/vpd/vpd_common.c:

  • Patch Set #8, Line 95: vpd_find_common

    maybe rename to vpd_find_from_blob, which is more descriptive than find_common.

  • Patch Set #8, Line 116: bufcpy

    we don't encourage acronyms in naming unless if it's very well known/trivial.

    Try something like

     vpd_copy_buffer or vpd_copy_string.

  • Patch Set #8, Line 122: if (size > (string_size + 1)) {

    what about

     int copy_size = MIN(size - 1, string_size);
     memcpy(buffer, string_addr, copy_size);
     buffer[copy_size] = '\0';

To view, visit change 34634. To unsubscribe, or for help writing mail filters, visit settings.