[flashrom] Libflashrom draft patch - initial review
Антон Кочков
anton.kochkov at gmail.com
Fri Jun 19 13:43:08 CEST 2015
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 <urjaman at gmail.com> wrote:
> Hi,
>
>
> On Fri, Jun 12, 2015 at 12:26 AM, Åukasz Dmitrowski
> <lukasz.dmitrowski at gmail.com> 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 abou¤ ¶ì€U¶ì€U¼; U
ó Ú[ z4²² ÁË Á˶ì€U ÁË ¤ ~4 ¾µƒU¶ì€U¼; U
ó T Ä\ {4²² QáÎ |" j¶ì€U ÁË ¤ ‘
¶ì€U¶ì€U¼; U
ó ! ] |4²² uÕÏ Qáζì€U QáÎ ¤ ¡˜ ¾µƒU¶ì€ULŠkS P
ó ª H] 4²² Eèâ |" j¶ì€U uÕÏ ¤ 5] ¶ì€U·ì€U
ŽkS 0
ó F ò^ €4²² 9Ë Eèâ¶ì€U Eèâ ¤ f> ¾µƒU·ì€ULŠkS
ó 8` 4²² 9Ë |" j·ì€U 9Ë ¤ × ·ì€U·ì€U
ŽkS
ó <` ‚4²² 9Ë 9Ë·ì€U 9Ë ¤ Ìs ¾µƒU·ì€UTŠkS è9
ó = >` ƒ4²² Ù£ |" j·ì€U 9Ë ¤ ·ïx ·ì€U¸ì€UfŽkS x<
ó {g „4²² Å~ Ù£·ì€U Ù£ íAf
SU¿„U¿„U j ¤
ó Y& …4²² ðiƒpðiƒpðV`n¸ì€U Å~ € ˜öŠUiTUiTUƒ0 §
ó ¯ ' 0 H / H0 x x0 hT ÐKhÐKh¬vÁŽ˜öŠU¬vÁŽ ¤ Ì ½µƒU½U¬C U
ó " "T `×— |Â1½U`)• ¤f øŽÛ½—ƒUÃì€UºB U j Èí
ó ¹= ð ˆ4²² Í÷< @‘‘E»ì€U ]Úº €f D
¿„U¿„U¿„U j
ó '~ ‰4²² ðiƒpðiƒpðùStÃì€U Í÷< ¤ o Ãì€UÃì€UB U
ó o ‹4²² ñë= ñë=Ãì€U ñë= ¤ i Ãì€UÃì€UœB U
ó o Œ4²² ñë= ñë=Ãì€U ñë= -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 <nico.huber at secunet.com> 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 <nico.huber at secunet.com> 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
>> flashrom at flashrom.org
>> http://www.flashrom.org/mailman/listinfo/flashrom
>
> --
> Urja Rannikko
>
> _______________________________________________
> flashrom mailing list
> flashrom at flashrom.org
> http://www.flashrom.org/mailman/listinfo/flashrom
More information about the flashrom
mailing list