From: Joel Brobecker <brobecker@adacore.com>
To: Pedro Alves <palves@redhat.com>
Cc: Yao Qi <yao@codesourcery.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH 2/4] Error on bad count number
Date: Thu, 06 Mar 2014 13:24:00 -0000 [thread overview]
Message-ID: <20140306132410.GE16858@adacore.com> (raw)
In-Reply-To: <53186842.9030704@redhat.com>
> I think that what exact number is used for error return
> is mostly irrelevant. The interface of get_number's value
> return alone is just not sufficient as is. Not changing
> the prototype, we could e.g. in addition of checking for
> zero return, have the callers check whether the passed in
> char pointer pointer advanced (and make sure get_number
> doesn't advance the pointer on invalid input). So the
> correct use becomes:
>
> p = arg;
> num = get_number (&p);
> if (num == 0 && p == arg)
> error (_("Bad number: '%s'"), arg);
>
> Sort of like strtol vs itoa.
There are not so many calls to that routine or (get_number_trailer),
so I think it would be easy to change the interface. But better yet,
why not throw an error? Most of the current uses just throw an error
right after when the returned value was 0. There are some uses where
there is no error handling, but I think they need to be reviewed as
they could actually benefit from it. After a quick glance, it wasn't
obvious to me whether there was a hole in the implementation or not.
--
Joel
next prev parent reply other threads:[~2014-03-06 13:24 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-05 12:49 [PATCH 0/4] Tweak get_number related code Yao Qi
2014-03-05 12:49 ` [OBV PATCH 1/4] Add a newline in output messages Yao Qi
2014-03-05 14:22 ` Joel Brobecker
2014-03-06 6:45 ` Yao Qi
2014-03-05 12:49 ` [PATCH 2/4] Error on bad count number Yao Qi
2014-03-05 14:29 ` Joel Brobecker
2014-03-05 15:59 ` Pedro Alves
2014-03-06 9:56 ` Yao Qi
2014-03-06 12:21 ` Pedro Alves
2014-03-06 13:24 ` Joel Brobecker [this message]
2014-03-07 9:40 ` Yao Qi
2014-03-07 10:53 ` Eli Zaretskii
2014-03-07 11:09 ` Yao Qi
2014-03-07 13:27 ` Pedro Alves
2014-03-07 13:50 ` Joel Brobecker
2014-03-10 2:01 ` Yao Qi
2014-03-06 13:26 ` Joel Brobecker
2014-03-05 12:49 ` [PATCH 4/4] Remove argument optional_p from get_tracepoint_by_number Yao Qi
2014-03-05 14:44 ` Joel Brobecker
2014-03-05 12:49 ` [PATCH 3/4] Handle parse number error in goto_bookmark_command Yao Qi
2014-03-05 14:38 ` Joel Brobecker
2014-03-06 7:10 ` Yao Qi
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=20140306132410.GE16858@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=palves@redhat.com \
--cc=yao@codesourcery.com \
/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