Messages in this thread | | | Date | Wed, 27 May 2015 14:53:20 +0900 | From | Sergey Senozhatsky <> | Subject | Re: [PATCH] zram: check comp algorithm availability earlier |
| |
On (05/27/15 12:51), Minchan Kim wrote: [..] > > @@ -378,6 +378,12 @@ static ssize_t comp_algorithm_store(struct device *dev, > > if (sz > 0 && zram->compressor[sz - 1] == '\n') > > zram->compressor[sz - 1] = 0x00; > > > > + if (!zcomp_available_algorithm(zram->compressor)) { > > + pr_err("Error: unavailable compression algorithm: %s\n", > > + zram->compressor); > > + len = -EINVAL; > > + } > > + > > I'm not against this patch because it's better than old. > But let's think more about the pr_err part. > > If user try to set wrong algo name, he can see EINVAL. > Isn't it enough? > > I think every sane admin can think he passed wrong argument > if he sees -EINVAL. > So, I don't think we need to emit pr_err in here. >
well, it's here simply to make failure investigation easier. one surely will know that supplied string was not recognized as a compression algorithm name, but what was it.. "$3 instead of $2... or, wait, did $i contain something wrong?". zram knew what was wrong.
/* and you asked to put this warn here in your previous email. */
sure, can remove it.
> The reason I am paranoid about that is that I really don't want > to argue with syslog info which is part of ABI or not in future. > If possible, I don't want to depend on pr_xxx. >
just for the record... I don't understand this part.
ok. I'll resend later today.
-ss
| |