On 07/03/2012 05:38 PM, Tom Rini wrote:
On Tue, Jul 03, 2012 at 05:07:04PM -0600, Stephen Warren wrote:
On 07/03/2012 04:33 PM, Tom Rini wrote:
On Wed, Jul 04, 2012 at 12:31:14AM +0200, Marek Vasut wrote:
Dear Tom Rini,
[...]
>>> + debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf); >>> + run_command(cmd_buf, 0); >> >> Holy Moly ... can we not make this into simple calls to those >> subsystems ? Instead invoking command is crazy ;-) > > Are they really simple? There's a few other places we do this, and so > long as it's documented that DFU depends on CONFIG_FAT_WRITE for > writing to fat and so forth.
Well ain't it easier to call fat_write() or similar?
Assuming that most of the logic in do_fat_fswrite is needed, no. And I think a good portion of it is, at first glance at least.
Abstracting it out into a function won't cut it?
My inclination would be to say that seems a bit sillier than just using run_command(...) to call the existing command.
Abstracting into a function definitely means the compiler will be able to type-check all the arguments to the function. Is the same true using run_command; I assume everything gets serialized to an array of strings and hence any validation is deferred until run-time then?
string is constructed with sprintf and that gives us some type checking. Only cast today is buf to an unsigned int. And the information either comes from tools that should/must have done sanity checking or from the 'dfu' command which again should (didn't verify that patch yet) have done sanity checking to get us this far.
So that checks the parameters against the sprintf format string, but what checks that you chose the right format string for the command?