Hi! I've done also some rebasing of the libflashrom patches before (on top of the layout branch from the stefanct) https://github.com/XVilka/flashrom/commits/libflashrom
May be you'll find something useful here. Best regards, Anton Kochkov. On Thu, Jun 18, 2015 at 7:45 AM, Urja Rannikko <[email protected]> wrote: > Hi, > > > On Fri, Jun 12, 2015 at 12:26 AM, Łukasz Dmitrowski > <[email protected]> wrote: >> Hello, >> >> My name is Łukasz Dmitrowski and I am one of coreboot/flashrom GSoC students >> this year. You can read about my project here: >> http://blogs.coreboot.org/blog/author/dmitro/. >> >> One of my first steps is updating and extending Nico Hubers libflashrom >> patch. Work is still in progress, but some changes are already made, so I >> thought that it will be good idea to send a patch just for review to know if >> I am going in a good direction. I want to improve my skills and provide best >> code quality as I can, so I am open for criticism - I am here to learn :) >> > I'm here essentially trying out reviewing stuff, so expect more from > somebody a bit more experienced, but I heard my comments would be > useful, so moving on :) > >> Patch summary: >> > First thing i have to say about the patch is that it has been > malformed, most likely line wrapped by your email client. If you cant > properly include the patch inline, attaching a .patch would be okay. > (Also turn off that HTML output if you can... although it wasnt used > for anything, but still.). So I didnt do any compile testing on this > stuff because of this. > >> Makefile: I added cli_common.o and print.o to LIB_OBJS - it solves undefined >> references in current libflashrom version, but spoils standard flashrom >> build with 'make', I did not investigated it yet as my focus is to extend >> library with new functions and update already existing, need to discuss it >> with my mentor >> >> cli_classic.c, cli_output.c, flash.h: Only Nico Hubers changes here, didn't >> touch it > I might comment on any part of the patch because I havent reviewed the > original and it's only about the code. > >> >> libflashrom.c, libflashrom.h: I added 8 functions and made a few changes to >> existing ones, as some of used functions are static, or does not exist. >> >> New functions: >> >> int fl_supported_programmers(const char **supported_programmers); >> int fl_supported_flash_chips(fl_flashchip_info_t *fchips); >> int fl_supported_boards(fl_board_info_t *boards); >> int fl_supported_chipsets(fl_chipset_info_t *chipsets); >> int fl_supported_programmers_number(); >> int fl_supported_flash_chips_number(); >> int fl_supported_boards_number(); >> int fl_supported_chipsets_number(); >> >> New types: >> fl_flashchip_info_t - smaller version of struct flashchip >> fl_board_info_t - smaller version of struct board_info >> fl_chipset_info_t - smaller version of struct penable >> fl_test_state - copy of enum test_state >> >> So we have 4 functions that return a list of supported programmers, chips, >> boards and chipsets. Next 4 functions return number of supported hardware of >> certain type. For example, to obtain a list of supported boards, you can >> create an array of structures fl_board_info of suitable size (the size is >> known - fl_supported_boards_number()), then pass it to >> fl_supported_boards(fl_board_info_t *boards) and you will have it filled >> with data. > My opinion is that the _number functions make the API unnecessarily > complex to use. > I'm suggesting like this: > fl_flashchip_info_t * fl_supported_flash_chips(void); > Have the call allocate the table using a not specified allocator > (really just malloc for now, but we're not telling), and freed with a > function like > int fl_support_info_free(void*p); > This would allow implementation using const tables or allocation, > whatever we like. > (if (p == const_table) return 0; in our _free() if mixed.) > >> >> Regarding changes in Nico Hubers functions: I made temporary comments for >> every change and marked it with LD_CHANGE_NOTE. Every comment contains >> previous code and reason why it has been commented out. >> >> Thanks in advance for your insights! >> >> Regards, >> Lukasz Dmitrowski >> >> --- >> >> Signed-off-by: Łukasz Dmitrowski <[email protected]> >> >> Index: Makefile >> =================================================================== >> --- Makefile (revision 1891) >> +++ Makefile (working copy) >> @@ -374,7 +374,7 @@ >> >> ############################################################################### >> # Library code. >> >> -LIB_OBJS = layout.o flashrom.o udelay.o programmer.o helpers.o >> +LIB_OBJS = libflashrom.o layout.o flashrom.o udelay.o programmer.o >> helpers.o cli_common.o print.o >> >> >> ############################################################################### >> # Frontend related stuff. >> Index: cli_classic.c >> =================================================================== >> --- cli_classic.c (revision 1891) >> +++ cli_classic.c (working copy) >> @@ -30,6 +30,7 @@ >> #include "flash.h" >> #include "flashchips.h" >> #include "programmer.h" >> +#include "libflashrom.h" >> >> static void cli_classic_usage(const char *name) >> { >> @@ -135,6 +136,8 @@ >> char *tempstr = NULL; >> char *pparam = NULL; >> >> + fl_set_log_callback((fl_log_callback_t *)&fl_print_cb); >> + >> print_version(); >> print_banner(); >> >> Index: cli_output.c >> =================================================================== >> --- cli_output.c (revision 1891) >> +++ cli_output.c (working copy) >> @@ -71,9 +71,8 @@ >> #endif /* !STANDALONE */ >> >> /* Please note that level is the verbosity, not the importance of the >> message. */ >> -int print(enum msglevel level, const char *fmt, ...) >> +int fl_print_cb(enum msglevel level, const char *fmt, va_list ap) >> { >> - va_list ap; >> int ret = 0; >> FILE *output_type = stdout; >> >> @@ -81,9 +80,7 @@ >> output_type = stderr; >> >> if (level <= verbose_screen) { >> - va_start(ap, fmt); >> ret = vfprintf(output_type, fmt, ap); >> - va_end(ap); >> /* msg_*spew often happens inside chip accessors in possibly >> * time-critical operations. Don't slow them down by flushing. */ >> if (level != MSG_SPEW) >> @@ -91,9 +88,7 @@ >> } >> #ifndef STANDALONE >> if ((level <= verbose_logfile) && logfile) { >> - va_start(ap, fmt); >> ret = vfprintf(logfile, fmt, ap); >> - va_end(ap); >> if (level != MSG_SPEW) >> fflush(logfile); >> } >> Index: flash.h >> =================================================================== >> --- flash.h (revision 1891) >> +++ flash.h (working copy) >> @@ -31,6 +31,7 @@ >> #include <stdint.h> >> #include <stddef.h> >> #include <stdbool.h> >> +#include <stdarg.h> >> #if IS_WINDOWS >> #include <windows.h> >> #undef min >> @@ -317,6 +318,7 @@ >> MSG_DEBUG2 = 4, >> MSG_SPEW = 5, >> }; >> +int fl_print_cb(enum msglevel level, const char *fmt, va_list ap); >> /* Let gcc and clang check for correct printf-style format strings. */ >> int print(enum msglevel level, const char *fmt, ...) >> #ifdef __MINGW32__ >> Index: libflashrom.c >> =================================================================== >> --- libflashrom.c (revision 0) >> +++ libflashrom.c (working copy) >> @@ -0,0 +1,603 @@ >> +/* >> + * This file is part of the flashrom project. >> + * >> + * Copyright (C) 2012 secunet Security Networks AG >> + * (Written by Nico Huber <[email protected]> for secunet) >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 >> USA >> + */ >> +/** >> + * @mainpage >> + * >> + * Have a look at the Modules section for a function reference. >> + */ >> + >> +#include <stdlib.h> >> +#include <string.h> >> +#include <stdarg.h> >> + >> +#include "flash.h" >> +#include "programmer.h" >> +#include "libflashrom.h" >> + >> +/** >> + * @defgroup fl-general General >> + * @{ >> + */ >> + >> +/** Pointer to log callback function. */ >> +static fl_log_callback_t *fl_log_callback = NULL; >> + >> +/** >> + * @brief Initialize libflashrom. >> + * >> + * @param perform_selfcheck If not zero, perform a self check. >> + * @return 0 on success >> + */ >> +int fl_init(const int perform_selfcheck) >> +{ >> + if (perform_selfcheck && selfcheck()) >> + return 1; >> + myusec_calibrate_delay(); >> + return 0; >> +} >> + >> +/** >> + * @brief Shut down libflashrom. >> + * @return 0 on success >> + */ >> +int fl_shutdown(void) >> +{ >> + return 0; /* TODO: nothing to do? */ > I'm pretty sure there is, but need to investigate more. > >> +} >> + >> +/* TODO: fl_set_loglevel()? do we need it? >> + For now, let the user decide in his callback. */ >> + >> +/** >> + * @brief Set the log callback function. >> + * >> + * Set a callback function which will be invoked whenever libflashrom wants >> + * to output messages. This allows frontends to do whatever they see fit >> with >> + * such messages, e.g. write them to syslog, or to file, or print them in a >> + * GUI window, etc. >> + * >> + * @param log_callback Pointer to the new log callback function. >> + */ >> +void fl_set_log_callback(fl_log_callback_t *const log_callback) >> +{ >> + fl_log_callback = log_callback; >> +} >> +/** @private */ >> +int print(const enum msglevel level, const char *const fmt, ...) >> +{ >> + if (fl_log_callback) { >> + int ret; >> + va_list args; >> + va_start(args, fmt); >> + ret = fl_log_callback(level, fmt, args); >> + va_end(args); >> + return ret; >> + } >> + return 0; >> +} >> + >> +/** @} */ /* end fl-general */ >> + >> + >> + >> +/** >> + * @defgroup fl-query Querying >> + * @{ >> + */ >> + >> +int fl_supported_programmers(const char **supported_programmers) >> +{ >> + int ret = 0; >> + enum programmer p = 0; >> + >> + if (supported_programmers != NULL) { >> + for (; p < PROGRAMMER_INVALID; ++p) >> + supported_programmers[p] = >> programmer_table[p].name; >> + } else { >> + ret = 1; >> + } >> + >> + return ret; >> +} >> + >> +int fl_supported_programmers_number() >> +{ >> + return PROGRAMMER_INVALID; >> +} >> + >> +int fl_supported_flash_chips(fl_flashchip_info_t *fchips) >> +{ >> + int ret = 0; >> + int i = 0; >> + >> + if (fchips != NULL) { >> + for (; i < flashchips_size; ++i) { >> + fchips[i].vendor = flashchips[i].vendor; >> + fchips[i].name = flashchips[i].name; >> + fchips[i].tested.erase = >> flashchips[i].tested.erase; >> + fchips[i].tested.probe = >> flashchips[i].tested.probe; >> + fchips[i].tested.read = flashchips[i].tested.read; >> + fchips[i].tested.write = >> flashchips[i].tested.write; >> + fchips[i].total_size = flashchips[i].total_size; >> + } >> + } else { >> + ret = 1; >> + } >> + >> + return ret; >> +} >> + >> +int fl_supported_flash_chips_number() >> +{ >> + return flashchips_size; >> +} >> + >> +int fl_supported_boards(fl_board_info_t *boards) >> +{ >> + int ret = 0; >> + const struct board_info *binfo = boards_known; >> + >> + if (boards != NULL) { >> + while (binfo->vendor != NULL) { >> + boards->vendor = binfo->vendor; >> + boards->name = binfo->name; >> + boards->working = binfo->working; >> + ++binfo; >> + ++boards; >> + } >> + } else { >> + ret = 1; >> + } >> + >> + return ret; >> +} >> + >> +int fl_supported_boards_number() >> +{ >> + int boards_number = 0; >> + const struct board_info *binfo = boards_known; >> + >> + while (binfo->vendor != NULL) { >> + ++boards_number; >> + ++binfo; >> + } >> + >> + return boards_number; >> +} >> + >> +int fl_supported_chipsets(fl_chipset_info_t *chipsets) >> +{ >> + int ret = 0; >> + const struct penable *chipset = chipset_enables; >> + >> + if (chipsets != NULL) { >> + while (chipset->vendor_name != NULL) { >> + chipsets->vendor = chipset->vendor_name; >> + chipsets->chipset = chipset->device_name; >> + chipsets->status = chipset->status; >> + ++chipset; >> + ++chipsets; > These names are confusing (just FYI), i first thought we're counting > chipsets here.. >> + } >> + } else { >> + return ret; >> + } >> + >> + return ret; >> +} >> + >> +int fl_supported_chipsets_number() >> +{ >> + int chipsets_number = 0; >> + const struct penable *chipset = chipset_enables; >> + >> + while (chipset->vendor_name != NULL) { >> + ++chipsets_number; >> + ++chipset; >> + } >> + >> + return chipsets_number; >> +} >> + >> +/** @} */ /* end fl-query */ >> + >> + >> + >> +/** >> + * @defgroup fl-prog Programmers >> + * @{ >> + */ >> + >> +/** >> + * @brief Initialize the specified programmer. >> + * >> + * @param prog_name Name of the programmer to initialize. >> + * @param prog_param Pointer to programmer specific parameters. >> + * @return 0 on success >> + */ >> +int fl_programmer_init(const char *const prog_name, const char *const >> prog_param) >> +{ >> + unsigned prog; >> + >> + for (prog = 0; prog < PROGRAMMER_INVALID; prog++) { >> + if (strcmp(prog_name, programmer_table[prog].name) == 0) >> + break; >> + } >> + if (prog >= PROGRAMMER_INVALID) { >> + msg_ginfo("Error: Unknown programmer \"%s\". Valid choices >> are:\n", prog_name); >> + list_programmers_linebreak(0, 80, 0); >> + return 1; >> + } >> + return programmer_init(prog, prog_param); >> +} >> + >> +/** >> + * @brief Shut down the initialized programmer. >> + * >> + * @return 0 on success >> + */ >> +int fl_programmer_shutdown(void) >> +{ >> + return programmer_shutdown(); >> +} >> + >> +/* TODO: fl_programmer_capabilities()? */ >> + >> +/** @} */ /* end fl-prog */ >> + >> + >> + >> +/** >> + * @defgroup fl-flash Flash chips >> + * @{ >> + */ >> + >> +/** >> + * @brief Probe for a flash chip. >> + * >> + * Probes for a flash chip and returns a flash context, that can be used >> + * later with flash chip and @ref fl-ops "image operations", if exactly one >> + * matching chip is found. >> + * >> + * @param[out] flashctx Points to a pointer of type fl_flashctx_t that will >> + * be set if exactly one chip is found. *flashctx has >> + * to be freed by the caller with @ref >> fl_flash_release. >> + * @param[in] chip_name Name of a chip to probe for, or NULL to probe for >> + * all known chips. >> + * @return 0 on success, >> + * 3 if multiple chips were found, >> + * 2 if no chip was found, >> + * or 1 on any other error. >> + */ > Need a way to output multiple flash chips from here, it shouldnt be an > error, just a > "pick from these" dialog and continue with "OK", or something like that. > Maybe add int *chip_count as a parameter and the set pointer will > point to an array of flashctx with the int set to the count of found > chips. > (Hopefully I'm not saying totally obvious stuff here...) > >> +int fl_flash_probe(fl_flashctx_t **const flashctx, const char *const >> chip_name) >> +{ >> + int i, ret = 2; >> + fl_flashctx_t second_flashctx = { 0, }; >> + >> + chip_to_probe = chip_name; /* chip_to_probe is global in flashrom.c >> */ >> + >> + *flashctx = malloc(sizeof(**flashctx)); >> + if (!*flashctx) >> + return 1; >> + memset(*flashctx, 0, sizeof(**flashctx)); >> + >> + /* LD_CHANGE_NOTE for (i = 0; i < registered_programmer_count; ++i) >> { >> + * >> + * Reason of change: registered_programmer_count does not exist, I >> assume that a proper one is >> + * now registered_master_count - I will check and confirm >> + */ >> + for (i = 0; i < registered_master_count; ++i) { >> + int flash_idx = -1; >> + /* LD_CHANGE_NOTE if (!ret || (flash_idx = >> probe_flash(®istered_programmers[i], 0, *flashctx, 0)) != -1) { >> + * >> + * Reason of change: registered_programmers does not exist, >> I assume that a proper one is >> + * now registered_masters - I will check and confirm >> + */ >> + if (!ret || (flash_idx = >> probe_flash(®istered_masters[i], 0, *flashctx, 0)) != -1) { >> + ret = 0; >> + /* We found one chip, now check that there is no >> second match. */ >> + /* LD_CHANGE_NOTE if >> (probe_flash(®istered_programmers[i], flash_idx + 1, &second_flashctx, 0) >> != -1) { >> + * >> + * Reason of change: registered_programmers does >> not exist, I assume that a proper one is >> + * now registered_masters - I will check and >> confirm >> + * >> + */ >> + if (probe_flash(®istered_masters[i], flash_idx + >> 1, &second_flashctx, 0) != -1) { >> + ret = 3; >> + break; >> + } >> + } >> + } >> + if (ret) { >> + free(*flashctx); >> + *flashctx = NULL; >> + } >> + return ret; >> +} >> + >> +/** >> + * @brief Returns the size of the specified flash chip in bytes. >> + * >> + * @param flashctx The queried flash context. >> + * @return Size of flash chip in bytes. >> + */ >> +size_t fl_flash_getsize(const fl_flashctx_t *const flashctx) >> +{ >> + return flashctx->chip->total_size << 10; >> +} >> + >> +/** @private */ >> +int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents, >> uint8_t *newcontents); >> +/** @private */ >> +/* LD_CHANGE_NOTE void emergency_help_message(void); >> + * >> + * Reason of change: This has been commented out as emergency_help_message >> is static > Maybe have some other method to indicate that the UI should display an > emergency help message, maybe ret = 2. Same comment for the _write > version, but that already has ret values for these. > >> + */ >> +/** >> + * @brief Erase the specified ROM chip. >> + * >> + * @param flashctx The context of the flash chip to erase. >> + * @return 0 on success. >> + */ >> +int fl_flash_erase(fl_flashctx_t *const flashctx) >> +{ >> + const size_t flash_size = flashctx->chip->total_size * 1024; >> + >> + int ret = 0; >> + >> + uint8_t *const newcontents = malloc(flash_size); >> + if (!newcontents) { >> + msg_gerr("Out of memory!\n"); >> + return 1; >> + } >> + uint8_t *const oldcontents = malloc(flash_size); >> + if (!oldcontents) { >> + msg_gerr("Out of memory!\n"); >> + free(newcontents); >> + return 1; >> + } >> + >> + if (flashctx->chip->unlock) >> + flashctx->chip->unlock(flashctx); >> + >> + /* Assume worst case for old contents: All bits are 0. */ >> + memset(oldcontents, 0x00, flash_size); >> + /* Assume best case for new contents: All bits should be 1. */ >> + memset(newcontents, 0xff, flash_size); >> + /* Side effect of the assumptions above: Default write action is >> erase >> + * because newcontents looks like a completely erased chip, and >> + * oldcontents being completely 0x00 means we have to erase >> everything >> + * before we can write. >> + */ >> + >> + if (erase_and_write_flash(flashctx, oldcontents, newcontents)) { >> + /* FIXME: Do we really want the scary warning if erase >> failed? >> + * After all, after erase the chip is either blank or >> partially >> + * blank or it has the old contents. A blank chip won't >> boot, >> + * so if the user wanted erase and reboots afterwards, the >> user >> + * knows very well that booting won't work. >> + */ >> + /* LD_CHANGE_NOTE emergency_help_message(); >> + * >> + * Reason of change: This function is static >> + */ >> + ret = 1; >> + } >> + >> + free(oldcontents); >> + free(newcontents); >> + return ret; >> +} >> + >> +/** >> + * @brief Free a flash context. >> + * >> + * @param flashctx Flash context to free. >> + */ >> +void fl_flash_release(fl_flashctx_t *const flashctx) >> +{ >> + free(flashctx); >> +} >> + >> +/** @} */ /* end fl-flash */ >> + >> + >> + >> +/** >> + * @defgroup fl-ops Operations >> + * @{ >> + */ >> + >> +/** >> + * @brief Read the current image from the specified ROM chip. >> + * >> + * @param flashctx The context of the flash chip. >> + * @param buffer Target buffer to write image to. >> + * @param buffer_len Size of target buffer in bytes. >> + * @return 0 on success, >> + * 2 if buffer_len is to short for the flash chip's contents, >> + * or 1 on any other failure. >> + */ >> +int fl_image_read(fl_flashctx_t *const flashctx, void *const buffer, const >> size_t buffer_len) >> +{ >> + const size_t flash_size = flashctx->chip->total_size * 1024; >> + >> + int ret = 0; >> + >> + if (flashctx->chip->unlock) >> + flashctx->chip->unlock(flashctx); >> + >> + msg_cinfo("Reading flash... "); >> + if (flash_size > buffer_len) { >> + msg_cerr("Buffer to short for this flash chip (%u < >> %u).\n", >> + (unsigned int)buffer_len, (unsigned >> int)flash_size); >> + ret = 2; >> + goto _out; >> + } >> + if (!flashctx->chip->read) { >> + msg_cerr("No read function available for this flash >> chip.\n"); >> + ret = 1; >> + goto _out; >> + } >> + if (flashctx->chip->read(flashctx, buffer, 0, flash_size)) { >> + msg_cerr("Read operation failed!\n"); >> + ret = 1; >> + goto _out; >> + } >> +_out: >> + msg_cinfo("%s.\n", ret ? "FAILED" : "done"); >> + return ret; >> +} >> + >> +/** @private */ >> +/* LD_CHANGE_NOTE void nonfatal_help_message(void); >> + * >> + * Reason of change: This function is static >> + */ >> +/** >> + * @brief Write the specified image to the ROM chip. >> + * >> + * @param flashctx The context of the flash chip. >> + * @param buffer Source buffer to read image from. >> + * @param buffer_len Size of source buffer in bytes. >> + * @return 0 on success, >> + * 4 if buffer_len doesn't match the size of the flash chip, >> + * 3 if write was tried, but nothing has changed, >> + * 2 if write was tried, but flash contents changed, >> + * or 1 on any other failure. >> + */ >> +int fl_image_write(fl_flashctx_t *const flashctx, void *const buffer, const >> size_t buffer_len) >> +{ >> + const size_t flash_size = flashctx->chip->total_size * 1024; >> + >> + int ret = 0; >> + >> + if (buffer_len != flash_size) { >> + msg_cerr("Buffer size doesn't match size of flash chip (%u >> != %u)\n.", >> + (unsigned int)buffer_len, (unsigned >> int)flash_size); >> + return 4; >> + } >> + >> + uint8_t *const newcontents = buffer; >> + uint8_t *const oldcontents = malloc(flash_size); >> + if (!oldcontents) { >> + msg_gerr("Out of memory!\n"); >> + return 1; >> + } >> + if (fl_image_read(flashctx, oldcontents, flash_size)) { >> + ret = 1; >> + goto _free_out; >> + } >> + >> + /* LD_CHANGE_NOTE handle_romentries(flashctx, oldcontents, >> newcontents); >> + * >> + * Reason of change: This function does not exist. There is a need >> to investigate what was >> + * its purpose and how it can be replaced. >> + */ >> + >> + if (erase_and_write_flash(flashctx, oldcontents, newcontents)) { >> + msg_cerr("Uh oh. Erase/write failed. Checking if anything >> changed.\n"); >> + if (!flashctx->chip->read(flashctx, newcontents, 0, >> flash_size)) { >> + if (!memcmp(oldcontents, newcontents, flash_size)) >> { >> + msg_cinfo("Good. It seems nothing was >> changed.\n"); >> + /* LD_CHANGE_NOTE nonfatal_help_message(); >> + * >> + * Reason of change: This function is >> static >> + */ >> + ret = 3; >> + goto _free_out; >> + } >> + } >> + /* LD_CHANGE_NOTE emergency_help_message(); >> + * >> + * Reason of change: This function is static >> + */ >> + ret = 2; >> + goto _free_out; >> + } >> + >> +_free_out: >> + free(oldcontents); >> + return ret; >> +} >> + >> +/** @private */ >> +/* LD_CHANGE_NOTE int compare_range(const uint8_t *wantbuf, const uint8_t >> *havebuf, unsigned int start, unsigned int len); >> + * >> + * Reason of change: This function is static > For any of these "this is static"... then make it not static or figure > out a cleaner way of doing it, but yes i get this is an early patch. > >> + */ >> +/** >> + * @brief Verify the ROM chip's contents with the specified image. >> + * >> + * @param flashctx The context of the flash chip. >> + * @param buffer Source buffer to verify with. >> + * @param buffer_len Size of source buffer in bytes. >> + * @return 0 on success, >> + * 2 if buffer_len doesn't match the size of the flash chip, >> + * or 1 on any other failure. >> + */ >> +int fl_image_verify(fl_flashctx_t *const flashctx, void *const buffer, >> const size_t buffer_len) >> +{ >> + const size_t flash_size = flashctx->chip->total_size * 1024; >> + >> + int ret = 0; >> + >> + if (buffer_len != flash_size) { >> + msg_cerr("Buffer size doesn't match size of flash chip (%u >> != %u)\n.", >> + (unsigned int)buffer_len, (unsigned >> int)flash_size); >> + return 2; >> + } >> + >> + /* LD_CHANGE_NOTE uint8_t *const newcontents = buffer; - used only >> in handle_romentries() function >> + * >> + * Reason of change: This pointer is used only in handle_romentries >> function, which has been >> + * commented out >> + */ >> + uint8_t *const oldcontents = malloc(flash_size); >> + if (!oldcontents) { >> + msg_gerr("Out of memory!\n"); >> + return 1; >> + } >> + if (fl_image_read(flashctx, oldcontents, flash_size)) { >> + ret = 1; >> + goto _free_out; >> + } >> + >> + /* LD_CHANGE_NOTE handle_romentries(flashctx, oldcontents, >> newcontents); >> + * >> + * Reason of change: This function does not exist >> + */ >> + >> + msg_cinfo("Verifying flash... "); >> + >> + /* LD_CHANGE_NOTE ret = compare_range(newcontents, oldcontents, 0, >> flash_size); >> + * >> + * Reason of change: This function is static. For now replaced with >> ret = 1 as ret must be used >> + */ >> + ret = 1; >> + if (!ret) >> + msg_cinfo("VERIFIED.\n"); >> + >> +_free_out: >> + free(oldcontents); >> + return ret; >> +} >> + >> +/** @} */ /* end fl-ops */ >> Index: libflashrom.h >> =================================================================== >> --- libflashrom.h (revision 0) >> +++ libflashrom.h (working copy) >> @@ -0,0 +1,97 @@ >> +/* >> + * This file is part of the flashrom project. >> + * >> + * Copyright (C) 2012 secunet Security Networks AG >> + * (Written by Nico Huber <[email protected]> for secunet) >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 >> USA >> + */ >> + >> +#ifndef __LIBFLASHROM_H__ >> +#define __LIBFLASHROM_H__ 1 >> + >> +#include <stdarg.h> >> + >> +int fl_init(int perform_selfcheck); >> +int fl_shutdown(void); >> +/** @ingroup fl-general */ >> +typedef enum { /* This has to match enum msglevel. */ > Maybe you could just bring enum msglevel here from flash.h... > or have some other header for this printing stuff thats > used by both libflashrom.h and other parts of flashrom. > >> + FL_MSG_ERROR = 0, >> + FL_MSG_INFO = 1, >> + FL_MSG_DEBUG = 2, >> + FL_MSG_DEBUG2 = 3, >> + FL_MSG_SPEW = 4, >> +} fl_log_level_t; >> +/** @ingroup fl-general */ >> +typedef int(fl_log_callback_t)(fl_log_level_t, const char *format, >> va_list); >> +void fl_set_log_callback(fl_log_callback_t *); >> + >> +typedef enum { >> + FL_TESTED_OK = 0, >> + FL_TESTED_NT = 1, >> + FL_TESTED_BAD = 2, >> + FL_TESTED_DEP = 3, >> + FL_TESTED_NA = 4, >> +} fl_test_state; >> + >> +typedef struct { >> + const char *vendor; >> + const char *name; >> + unsigned int total_size; >> + >> + struct fl_tested { >> + fl_test_state probe; >> + fl_test_state read; >> + fl_test_state erase; >> + fl_test_state write; >> + } tested; >> +} fl_flashchip_info_t; >> + >> +typedef struct { >> + const char *vendor; >> + const char *name; >> + fl_test_state working; >> +} fl_board_info_t; >> + >> +typedef struct { >> + const char *vendor; >> + const char *chipset; >> + fl_test_state status; >> +} fl_chipset_info_t; >> + >> +int fl_supported_programmers(const char **supported_programmers); >> +int fl_supported_flash_chips(fl_flashchip_info_t *fchips); >> +int fl_supported_boards(fl_board_info_t *boards); >> +int fl_supported_chipsets(fl_chipset_info_t *chipsets); >> +int fl_supported_programmers_number(); >> +int fl_supported_flash_chips_number(); >> +int fl_supported_boards_number(); >> +int fl_supported_chipsets_number(); >> + >> +int fl_programmer_init(const char *prog_name, const char *prog_params); >> +int fl_programmer_shutdown(void); >> + >> +struct flashctx; >> +typedef struct flashctx fl_flashctx_t; >> +int fl_flash_probe(fl_flashctx_t **, const char *chip_name); >> +size_t fl_flash_getsize(const fl_flashctx_t *); >> +int fl_flash_erase(fl_flashctx_t *); >> +void fl_flash_release(fl_flashctx_t *); >> + >> +int fl_image_read(fl_flashctx_t *, void *buffer, size_t buffer_len); >> +int fl_image_write(fl_flashctx_t *, void *buffer, size_t buffer_len); >> +int fl_image_verify(fl_flashctx_t *, void *buffer, size_t buffer_len); >> + >> +#endif /* !__LIBFLASHROM_H__ */ >> >> _______________________________________________ >> flashrom mailing list >> [email protected] >> http://www.flashrom.org/mailman/listinfo/flashrom > > -- > Urja Rannikko > > _______________________________________________ > flashrom mailing list > [email protected] > http://www.flashrom.org/mailman/listinfo/flashrom _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
