One other additional thought regarding partial read/write: the current logic all will fail to erase&write all-zero newcontent. Instead, if the reader bails (read_all_first == 0), the copy_old_content() logic should still read old included regions. In other words, it should do the opposite of what it currently does. +Reardon
On Tue, Aug 12, 2014 at 8:53 PM, TR Reardon thomas_reardon@hotmail.com wrote:
Ok, sorry for my misunderstanding. But even with that change, copy_old_content() still gets called to copy the old non-readable regions. Shouldn't copy_old_content() just set oldcontent and newcontent to all-ones for excluded regions?
The unselected regions are not automatically skipped by the "do I need to erase and then update this block?" comparison logic in need_erase(), correct? At least, when I run it, the excluded regions are erased (rather than skipped), but then not written.
Anyway, I made one minor adjustment to your layout patches for partial read, else segfault:
diff --git a/flashrom.c b/flashrom.c index 27d1736..e3639ae 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1286,10 +1286,10 @@ int write_image_to_file(const unsigned char *buf, unsigned long size, const char l = get_next_included_romentry(0);
while (l != NULL) {
const char* name = (l->file[0] == '\0') ? l->name : l->file;
const char* name = (l->file == NULL || l->file[0] ==
'\0') ? l->name : l->file; unsigned int len = l->end - l->start + 1; msg_gdbg2("Writing "%s" to "%s" 0x%08x - 0x%08x (%uB)... ",
l->name, l->file, l->start, l->end, len);
l->name, name, l->start, l->end, len); if (write_buf_to_file(buf + l->start, len, name) != 0) { msg_gdbg2("failed.\n"); return 1;
+Reardon
On Tue, Aug 12, 2014 at 4:33 PM, Stefan Tauner stefan.tauner@alumni.tuwien.ac.at wrote:
On Mon, 11 Aug 2014 19:14:53 -0400 TR Reardon thomas_reardon@hotmail.com wrote:
I'd like to start experimenting with partial-write support, now that partial-read works well for me. But as of now doit() does a full read before the write, which of course fails. Any updates that I could test? Any useful log I can provide regarding successes?
Note, I now have a buspirate so I can do full writes to recover, as needed (this works successfully now, if slowly)
As I wrote in my previous mail, (after applying the layout patches) you can disable the full read in doit() by setting read_all_first to 0. There are no further changes yet, because it does not make much sense until the proposed ones are merged...
-- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
On Tue, 12 Aug 2014 21:53:57 -0400 TR Reardon thomas_reardon@hotmail.com wrote:
One other additional thought regarding partial read/write: the current logic all will fail to erase&write all-zero newcontent. Instead, if the reader bails (read_all_first == 0), the copy_old_content() logic should still read old included regions. In other words, it should do the opposite of what it currently does.
Thanks for your reports. It definitely sounds like something has gone wrong while rebasing... I'll look into the issue(s) in the next days.