[flashrom] Libflashrom draft patch - initial review
Urja Rannikko
urjaman at gmail.com
Thu Jun 18 06:45:16 CEST 2015
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 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 <lukasz.dmitrowski at gmail.com>
>
> 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 <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
More information about the flashrom
mailing list