Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1477111

Re: [PATCH v4 2/3] mmc: core: Factor out the alignment of erase size

Show all headers | View raw

On Tue, Sep 06, 2016 at 02:26:06PM +0800, Baolin Wang wrote:
> On 6 September 2016 at 12:34, Andreas Mohr <andi@lisas.de> wrote:
> >> -     to = from + nr;
> >> -
> >> -     if (to <= from)
> >> -             return -EINVAL;
> >
> > Hmm, this is swallowing -EINVAL behaviour
> > i.e., now possibly violating protocol?
> 
> I didn't see what situation will make variable 'to' is less than
> 'from' since I think variable 'nr' is always larger than 0, right? If
> so, we should remove this useless checking. Thanks.

Hmm, indeed, since all participating variables are unsigned,
the existing calculation should never hit this.
However, one could argue that this is an additional safeguard
against implementation source getting modified in a way
that will suddenly result in this pathologic case becoming true
(where a -EINVAL bailout surely will then pinpoint things
much more visibly for some users,
as opposed to potential data corruption or some such).



I have seen another change

> -	if (nr == 0)
> -		return 0;

where it gets moved out of common path
and into MMC_ERASE_ARG-specific branch
(likely because the subsequent common-path conditional of
    nr > rem
is deemed sufficient).

This seems to be again a change
where a simple yet crucial
device geometry calculation post-condition
(either to > from, or nr > 0)
is then not verified specifically/separately.

Ultimately, I am not sure whether or not
these (post-)conditions should be verified
in their most basic, simple form,
as an extra/separate verification step.

HTH,

Andreas

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v4 2/3] mmc: core: Factor out the alignment of erase size Baolin Wang <baolin.wang@linaro.org> - 2016-09-06 05:00 +0200
  Re: [PATCH v4 2/3] mmc: core: Factor out the alignment of erase size Andreas Mohr <andi@lisas.de> - 2016-09-06 06:40 +0200
    Re: [PATCH v4 2/3] mmc: core: Factor out the alignment of erase size Baolin Wang <baolin.wang@linaro.org> - 2016-09-06 08:30 +0200
      Re: [PATCH v4 2/3] mmc: core: Factor out the alignment of erase size Andreas Mohr <andi@lisas.de> - 2016-09-06 09:20 +0200
        Re: [PATCH v4 2/3] mmc: core: Factor out the alignment of erase size Baolin Wang <baolin.wang@linaro.org> - 2016-09-06 10:30 +0200
      Re: [PATCH v4 2/3] mmc: core: Factor out the alignment of erase size Adrian Hunter <adrian.hunter@intel.com> - 2016-09-06 10:00 +0200
        Re: [PATCH v4 2/3] mmc: core: Factor out the alignment of erase size Baolin Wang <baolin.wang@linaro.org> - 2016-09-06 10:30 +0200

csiph-web