From: Heiko Schocher <heiko.schocher@invitel.hu>
To: u-boot@lists.denx.de
Subject: [U-Boot] RFC: factor out common i2c code
Date: Tue, 23 Mar 2010 07:26:54 +0100 [thread overview]
Message-ID: <4BA85F2E.5070302@invitel.hu> (raw)
In-Reply-To: <20100322213152.B43464C022@gemini.denx.de>
Hello Wolfgang,
Wolfgang Denk wrote:
> Dear Frans Meulenbroeks,
>
> In message <ac9c93b11003221418j391cbf66jf0cb0688da84dae1@mail.gmail.com> you wrote:
>> In cmd_i2c the following snippet of code appears 6 times so it would
>> be a good candidate to factor out into a static helper function:
>
> Thanks for digging into this.
>
>> Now before I actually start doing that a few questions.
>> Should I factor out all lines and create a helper function with three
>> pointer arguments to contain the return values chip, devaddr and alen
>> (apart from argv and cmdptp which are input parameters).
>
> A function with 5 arguments makes not much sense for for such a
> relatively simple piece of code.
>
>> Or would it be better to only factor out only the alen code and have a
>> function like:
>> int get_alen(char *str, cmd_tbl_t *cmdtp)
>> {
>> int alen, j;
>> alen = 1;
>> for (j = 0; j < 8; j++) {
>> if str[j] == '.') {
>> alen = str[j+1] - '0';
>> if (alen > 3) {
>> cmd_usage(cmdtp);
>> return -1;
>> }
>
> Eventually you move the cmd_usage() usage callout of the function so
> you can get rid of the second argument.
>
>> and call it by something like
>>
>> alen = get_alen(argv[2]);
>> if (alen < 0) return alen; /* or return -1 */
>
> if (alen < 0) {
> cmd_usage(cmdtp);
> return -1;
> }
>
> or even use a common error exit like
>
> if (alen < 0)
> goto errout;
I vote for:
alen = get_alen(argv[2]);
if (alen < 0)
goto errout;
bye
Heiko
prev parent reply other threads:[~2010-03-23 6:26 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-22 21:18 [U-Boot] RFC: factor out common i2c code Frans Meulenbroeks
2010-03-22 21:31 ` Wolfgang Denk
2010-03-23 6:26 ` Heiko Schocher [this message]
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=4BA85F2E.5070302@invitel.hu \
--to=heiko.schocher@invitel.hu \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.