Re: [PATCH 5/5] sysctl: pass kernel pointers to ->proc_handler

On Wed, Apr 22, 2020 at 03:46:26AM +0100, Al Viro wrote:

quoted

Better allocate count + 1 bytes here, that way a lot of insanity in the
instances can be simply converted to snprintf().  Yes, I know it'll bring
the Church Of Avoiding The Abomination Of Sprintf out of the woodwork,
but...

FWIW, consider e.g. net/sunrpc/sysctl.c:

Nevermind that the read side should be simply
		int err = proc_douintvec(table, write, buffer, lenp, ppos);
		/* Display the RPC tasks on writing to rpc_debug */
		if (!err && strcmp(table->procname, "rpc_debug") == 0)
			rpc_show_tasks(&init_net);
		return err;
the write side would become
		len = snprintf(buffer, *lenp + 1, "0x%04x\n",
				*(unsigned int *)table->data);
		if (len > *lenp)
			len = *lenp;
		*lenp -= len;
		*ppos += len;
		return 0;
and I really wonder if lifting the trailing boilerplate into the caller would've
been better.  Note that e.g. gems like
                        if (!first)
                                err = proc_put_char(&buffer, &left, '\t');
                        if (err)
                                break;
                        err = proc_put_long(&buffer, &left, lval, neg);
                        if (err)
                                break;
are due to lack of snprintf-to-user; now, lose the "to user" part and we suddenly
can be rid of that stuff...

That sounds pretty sensible, but can we do that as an extra step?
That is in merge window N just move to passing kernel pointers, check
for fall out.  In merge window N + 1 start allocatin the extra byte and
switch at least the common helpers for snprintf?