Messages in this thread | | | Date | Tue, 19 May 2015 10:55:32 -0700 | From | Brian Norris <> | Subject | Re: [RFC] block: remove never-modified global variable |
| |
On Tue, May 19, 2015 at 10:34:56AM +0300, Boaz Harrosh wrote: > On 05/19/2015 10:19 AM, Boaz Harrosh wrote: > <> > > But specially now that you are unconditionally printing it. It is better > > to just combine the two statements. See suggested patch below: > > > > Actually we can even do better: > > diff --git a/block/partitions/check.c b/block/partitions/check.c > index 513c601..9bea23d 100644 > --- a/block/partitions/check.c > +++ b/block/partitions/check.c > @@ -160,35 +160,31 @@ check_partition(struct gendisk *hd, struct block_device *bdev) > > i = res = err = 0; > while (!res && check_part[i]) { > memset(state->parts, 0, state->limit * sizeof(state->parts[0])); > res = check_part[i++](state); > if (res < 0) { > /* We have hit an I/O error which we don't report now. > * But record it, and let the others do their job. > */ > err = res; > res = 0; > } > > } > > boaz> When we exit the loop res can only be (res >= 0) > > if (res > 0) { > printk(KERN_INFO "%s", state->pp_buf); > > free_page((unsigned long)state->pp_buf); > return state; > > boaz> When bigger we exit here > > } > if (state->access_beyond_eod) > err = -ENOSPC; > > boaz> So by this stage res can only be == 0. So we should just do: > > if (err) > /* The partition is unrecognized. So report I/O errors if there were any */ > - res = err;
^^ You don't want to kill this line, since we still return ERR_PRT(res). But otherwise, I think you're right.
> - if (res) { > - strlcat(state->pp_buf, > - " unable to read partition table\n", PAGE_SIZE); > - printk(KERN_INFO "%s", state->pp_buf); > - } > + printk(KERN_INFO "%s unable to read partition table\n", > + state->pp_buf); > > free_page((unsigned long)state->pp_buf); > free_partitions(state); > return ERR_PTR(res); > } > > ---- > Here is a proper diff:
I can send out a full patch, squashing my original + your fixup + my fixup, if that sounds OK.
Brian
| |