On Tue, 2018-03-06 at 19:11 +0100, Adam Borowski wrote: Thanks for the patch, my comments below.
> Attempting to print an object pointed to by a bad (usually ERR_PTR) > pointer > is a not so surprising error. Our code handles them inconsistently: > * two places print (null) if ptr<PAGE_SIZE > * one place prints (null) if abs(ptr)<PAGE_SIZE > * one place prints (null) only if !ptr > > Obviously, saying (null) for a small but non-0 value is misleading. > Thus, let's print: > * (null) for exactly 0 > * (err) if last page && abs(ptr)<=MAX_ERRNO > * (invalid) otherwise > First of all, this patch is much more arguable than the other one in your small series. "(invalid)" is invalid. Hint: there is a nice comment in the code why. I'm in principle not putting explanation here to insist people to eventually _read and understand_ the code before doing anything. Some comments below. > +#define BAD_PTR_STRING(x) (!(x) ? "(null)" : IS_ERR(x) ? "(err)" : > "(invalid)") It looks ugly. > /** > * simple_strtoull - convert a string to an unsigned long long > * @cp: The start of the string > @@ -588,7 +590,7 @@ char *string(char *buf, char *end, const char *s, > struct printf_spec spec) > size_t lim = spec.precision; > > if ((unsigned long)s < PAGE_SIZE) > - s = "(null)"; > + s = BAD_PTR_STRING(s); It doesn't make any sense before your patch 2. > if ((unsigned long)dn < PAGE_SIZE) > - return string(buf, end, "(null)", spec); > + return string(buf, end, BAD_PTR_STRING(dn), spec); It simple doesn't make sense. The idea is to do it below, in the pointer. These certain lines are going to be removed by my patch. > - return string(buf, end, "(null)", spec); > + return string(buf, end, BAD_PTR_STRING(ptr), spec); Doesn't make sense before your patch 2. > if ((unsigned long)save_str > (unsigned > long)-PAGE_SIZE > || (unsigned long)save_str < > PAGE_SIZE) > - save_str = "(null)"; > + save_str = BAD_PTR_STRING(save_str); This is perhaps one valid change in such situation. -- Andy Shevchenko <andriy.shevche...@linux.intel.com> Intel Finland Oy
