public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.u.boot@aribaud.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/5] Add bmp_layout module for accessing BMP header data
Date: Mon, 4 Feb 2013 22:26:28 +0100	[thread overview]
Message-ID: <20130204222628.545da91e@lilith> (raw)
In-Reply-To: <20130204192618.529EF200113@gemini.denx.de>

Hi Wolfgang,

On Mon, 04 Feb 2013 20:26:18 +0100, Wolfgang Denk <wd@denx.de> wrote:

> Dear Nikita Kiryanov,
> 
> In message <1359977979-28585-2-git-send-email-nikita@compulab.co.il> you wrote:
> > Currently code that displays BMP files does two things:
> > * assume that any address is a valid load address for a BMP
> > * access in-memory BMP header fields directly
> > 
> > Since some BMP header fields are 32 bit wide, this has a potential
> > for causing data aborts when these fields are placed in unaligned
> > addresses.
> > 
> > Create an API for safely accessing BMP header data, and compile it with
> > $(PLATFORM_NO_UNALIGNED) to give it the ability to emulate unaligned memory
> > accesses.
> 
> Frankly,  I think this is overkill.  U-Boot is a bootloader, and it is
> supposed to be lean and eficient.  We don't have all levels of safety
> systems and protective devices as in, for example, an aircraft.  You
> are supposed to know what you are doing, and if you ignore the rules,
> you will quickly see the results yourself.
> 
> There is plenty of other areas where correct opration requires certain
> alignments, and none of these are enforced by U-Boot.  And actually I
> think this is not only acceptable, but good as is.
> 
> "UNIX was not designed to stop you from doing stupid things,  because
> that would also stop you from doing clever things."       - Doug Gwyn
> 
> 
> You talk about BMP header - but we also have alignment requirements
> for image headers, well, even for a plain "md" or "mw" command.  And
> none of these provide any protection against accidsential (or
> intentional) access to unaligned addresses.
> 
> My recommendation is: just don;t do it, then.

The point about md not checking alignment is indeed valid: one should
know that a md.l requires a 4-byte-aligned address or it will abort.

OTOH, a cautious user may think that to ensure proper alignment, a BMP
should be loaded on a 4-byte boundary, but IIUC that it precisely what
will cause the load to fail, due to the sole 4-byte field of the BMP
header being misaligned by two bytes.

So if we leave BMP loading as it is now, the load address will need to
be 16-bit-but-not-32-bit-aligned, which is complicated enough to
require documentation.

Or, the BMP struct could be prepended with two bytes so that the
load address alignment requirement becomes a simple 4-byte boundary,
which most users are... bound... to choose naturally.

But ISTR the idea of prepending two bytes was already discussed and for
some reason it could not work. Jeroen?

> Best regards,
> 
> Wolfgang Denk

Amicalement,
-- 
Albert.

  reply	other threads:[~2013-02-04 21:26 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-04 11:39 [U-Boot] [PATCH 0/5] Create an API for safely accessing BMP header fields Nikita Kiryanov
2013-02-04 11:39 ` [U-Boot] [PATCH 1/5] Add bmp_layout module for accessing BMP header data Nikita Kiryanov
2013-02-04 19:26   ` Wolfgang Denk
2013-02-04 21:26     ` Albert ARIBAUD [this message]
2013-02-04 23:11       ` Wolfgang Denk
2013-02-05  8:07         ` Igor Grinberg
2013-02-05  9:57           ` Wolfgang Denk
2013-02-05 11:38             ` Igor Grinberg
2013-02-05 13:20               ` Wolfgang Denk
2013-02-05 14:49                 ` Igor Grinberg
2013-02-05 19:27                   ` Wolfgang Denk
2013-02-05  6:52     ` Nikita Kiryanov
2013-02-05  9:47       ` Wolfgang Denk
2013-02-04 11:39 ` [U-Boot] [PATCH 2/5] lcd: use bmp_layout API Nikita Kiryanov
2013-02-04 11:39 ` [U-Boot] [PATCH 3/5] cmd_bmp: " Nikita Kiryanov
2013-02-04 11:39 ` [U-Boot] [PATCH 4/5] video/cfb_console: " Nikita Kiryanov
2013-02-04 11:39 ` [U-Boot] [PATCH 5/5] video/bus_vcxk: " Nikita Kiryanov
2013-02-04 11:57 ` [U-Boot] [PATCH 0/5] Create an API for safely accessing BMP header fields Albert ARIBAUD
2013-02-04 12:37   ` Nikita Kiryanov
2013-02-04 14:19     ` Albert ARIBAUD
2013-02-04 15:07       ` Nikita Kiryanov
2013-02-04 15:25         ` Albert ARIBAUD
2013-02-04 20:48           ` Wolfgang Denk
2013-02-05  7:20           ` Nikita Kiryanov
2013-02-04 20:39       ` Wolfgang Denk
2013-02-05 14:58       ` Tom Rini
2013-02-05 19:37         ` Albert ARIBAUD
2013-02-04 20:35     ` Wolfgang Denk
2013-02-04 21:02       ` Jeroen Hofstee
2013-02-05  7:39         ` Igor Grinberg
2013-02-05  6:53 ` Igor Grinberg
2013-02-05  7:22   ` Nikita Kiryanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130204222628.545da91e@lilith \
    --to=albert.u.boot@aribaud.net \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox