Uh, forgot to add, that loading layout will be quite useful too, in the API and adding romentry_t* argument for all fl_image_* functions. Best regards, Anton Kochkov.
On Fri, Jun 19, 2015 at 2:43 PM, Антон Кочков <[email protected]> wrote: > 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
