Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Steven Rostedt <rostedt@goodmis.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        David Wysochanski <dwysocha@redhat.com>,
        Bruce Fields <bfields@fieldses.org>
Subject: Re: [PATCH v2 01/25] NFSD: Fix TP_printk() format specifier in
 trace_nfsd_dirent()
Thread-Topic: [PATCH v2 01/25] NFSD: Fix TP_printk() format specifier in
 trace_nfsd_dirent()
Thread-Index: AQHXR0uQjwrq7dbjxka4bUuLuYA4jKrgD64AgAFwTwCAXlOlgA==
Date:   Mon, 12 Jul 2021 15:18:05 +0000
Message-ID: <BC78B174-91E3-44C5-8B49-2C5F34BA8823@oracle.com>
References: <162083366966.3108.12581818416105328952.stgit@klimt.1015granger.net>
 <162083370248.3108.7424008399973918267.stgit@klimt.1015granger.net>
 <20210512122623.79ee0dda@gandalf.local.home>
 <238C0E2D-C2A4-4578-ADD2-C565B3B99842@oracle.com>
 <20210513105018.7539996a@gandalf.local.home>
In-Reply-To: <20210513105018.7539996a@gandalf.local.home>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?hJSKtF6KgiAxiYuuFIXARxko44BbSUtERodijNrzcmhz3Fqf7PhFcDgN1mXD?=
 =?us-ascii?Q?nFXQlyoL1CzXvassAe25fJLKkiCFv6/e9Gg3LKroS9zfvwmRqgt4zbgfllHi?=
 =?us-ascii?Q?C1bR3sGTLJnIJc1pCMComUykvgWEKUqRvxdEHPyAfOoqrX0eH+Weur8wXQKG?=
 =?us-ascii?Q?iKjYLBDjQE7Wf9Yi4NfVoVBqJ8C5FO9hoRmAJESK0AMORQY4I6sTW8ouWbP0?=
 =?us-ascii?Q?7uKYgvHAiwGIxn7VJdqIktr7bj63ZS/yFTB0yJ326UbWizneSo/q3Ojh+ks/?=
 =?us-ascii?Q?6cxLHq6NxQBWsfpicMf5eHG+8dGdrEw/GtNW3zmglkJj1ujr0OCRHz2NUsW3?=
 =?us-ascii?Q?DRXz5c5myspfAs3jAwsMtHCKjbQ7+kkWIDNSedNuhqEJUxTWQmMw02LoNy6H?=
 =?us-ascii?Q?qbmhq732pLp1ebv7gEnRdLH+ThVuENt7Z4iGv5KeuG6J9CzZoC36p8ACXw2W?=
 =?us-ascii?Q?VY3g1uFAyJy/tHoXfQhIhYiw6dn6+COnRy4qi4+vu8dKQeEiPYQH38axM5Cl?=
 =?us-ascii?Q?P8Lw/ZPf5co2ELSJ4jouzU8p1IZ6XWAzLdY5KRmHv3NNABa77Jqv5FqswB+k?=
 =?us-ascii?Q?uptprpGE3WY8XjRjDuF1bPbsx9ohHB7YJQqm49cDNNLrc1RLv6lxhOfElzfc?=
 =?us-ascii?Q?alUeP82Bt8IlY9dNQKqsSM2BWswPEgFKIcIkqi99BqpxtRDhBIdqKpzLV8Ti?=
 =?us-ascii?Q?RlJ8GnYDgtgJCylUuxz+LrLkIHPtWlqJg8JZHKrtcweyfsqGqiX0DXj4Hg3r?=
 =?us-ascii?Q?k89UCi9aQSsd7nyDpIYaXNccVKZBD8LCYk38Yzzyt5qmhg5TV4Jtb4uKttVU?=
 =?us-ascii?Q?OMI3jWSPB7iuB8cXBSNYQuDpK5uH+LhjIgKCrPyCI2ZgJHEfJaK68nVDD+rL?=
 =?us-ascii?Q?fIolm+xU0E708Nmw4PVanpyuOhc6L/37B6JI0OsYzatOcf56aYTYZOvJxv+r?=
 =?us-ascii?Q?Ev+XdXE0B3+zPJmn2V0CRd/ohJgVuqzG96lMI/sdynSjvs810EpW3pzIll08?=
 =?us-ascii?Q?vlTTUpDTT/whs+LcshDT2C3XEoYUta+5EILfdNqOebXoVgXPbiSQIKHQNpud?=
 =?us-ascii?Q?G3BQnwRgt48BctBW/O7EwpImIxMAduA3gsJmeOGPxD/0HElVcTXub26jdxLV?=
 =?us-ascii?Q?TC58PYNyIkEMikoZ710bjE3voR4WD54Y72nnDltN/L+wTVM3LAxRvjDu4me5?=
 =?us-ascii?Q?WgLozMAdPi8OWOu3UhFzEf+BfYWAahqo4MqM9PY7ILysXLTDtOCGukVguSJ4?=
 =?us-ascii?Q?fivjq7bUTZefW6dGo4wrlUTe/EznuIsivgXmvJ+rSyNJR1hpI8G8s146wz02?=
 =?us-ascii?Q?Zx31XC1+jPqnAvfXpOWy6kaj?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-ID: <E29CE31FBC73AD42BC02BCA16A584FC9@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4688.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5677736d-3bcc-416d-ccf6-08d945483fdc
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Jul 2021 15:18:05.9937
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 9KEceFaVOT/DStcf/4yC6y0jb6XDPRZ8qX32/kxaBPU5E51o0BYEuYz7OS3u2Fwo+ATcEGwCWF3xfE8S/FlhHA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4290
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10043 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 adultscore=0
 phishscore=0 spamscore=0 bulkscore=0 mlxlogscore=999 mlxscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104190000 definitions=main-2107120119
X-Proofpoint-GUID: sS7nSEtebON_0-vzHN9dPoM6kZobGNcF
X-Proofpoint-ORIG-GUID: sS7nSEtebON_0-vzHN9dPoM6kZobGNcF
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

Hello Steven-

> On May 13, 2021, at 10:50 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>=20
> On Wed, 12 May 2021 16:52:05 +0000
> Chuck Lever III <chuck.lever@oracle.com> wrote:
>=20
>> The underlying need is to support non-NUL-terminated C strings.
>>=20
>> I assumed that since the commentary around 9a6944fee68e claims
>> the proper way to trace C strings is to use __string and friends,
>> and those do not support non-NUL-terminated strings, that such
>> strings are really not first-class citizens. Thus I concluded
>> that my use of '%.*s' was incorrect.
>>=20
>> Having some __string-style helpers that can deal with such
>> strings would be valuable.
>=20
> I guess the best I can do is a strncpy version, that will add the '\0' in
> the ring buffer. That way we don't need to save the length as well (lengt=
h
> would need to be at least 4 bytes, where as '\0' is one).
>=20
> Something like this?

I don't see this change in v5.14-rc1.


> I added "__string_len()" and "__assign_str_len()". You use them just like
> __string() and __assign_str() but add a max length that you want to use
> (although, it will always allocate "len" regardless if the string is
> smaller). Then use __get_str() just like you use __string().
>=20
> Would something like that work?
>=20
> -- Steve
>=20
> diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
> index 8268bf747d6f..7ab23535a0c8 100644
> --- a/include/trace/trace_events.h
> +++ b/include/trace/trace_events.h
> @@ -102,6 +102,9 @@ TRACE_MAKE_SYSTEM_STR();
> #undef __string
> #define __string(item, src) __dynamic_array(char, item, -1)
>=20
> +#undef __string_len
> +#define __string_len(item, src, len) __dynamic_array(char, item, -1)
> +
> #undef __bitmask
> #define __bitmask(item, nr_bits) __dynamic_array(char, item, -1)
>=20
> @@ -197,6 +200,9 @@ TRACE_MAKE_SYSTEM_STR();
> #undef __string
> #define __string(item, src) __dynamic_array(char, item, -1)
>=20
> +#undef __string_len
> +#define __string_len(item, src, len) __dynamic_array(char, item, -1)
> +
> #undef __bitmask
> #define __bitmask(item, nr_bits) __dynamic_array(unsigned long, item, -1)
>=20
> @@ -444,6 +450,9 @@ static struct trace_event_functions trace_event_type_=
funcs_##call =3D {	\
> #undef __string
> #define __string(item, src) __dynamic_array(char, item, -1)
>=20
> +#undef __string_len
> +#define __string_len(item, src, len) __dynamic_array(char, item, -1)
> +
> #undef __bitmask
> #define __bitmask(item, nr_bits) __dynamic_array(unsigned long, item, -1)
>=20
> @@ -492,6 +501,9 @@ static struct trace_event_fields trace_event_fields_#=
#call[] =3D {	\
> #define __string(item, src) __dynamic_array(char, item,			\
> 		    strlen((src) ? (const char *)(src) : "(null)") + 1)
>=20
> +#undef __string_len
> +#define __string_len(item, src, len) __dynamic_array(char, item, (len) +=
 1)
> +
> /*
>  * __bitmask_size_in_bytes_raw is the number of bytes needed to hold
>  * num_possible_cpus().
> @@ -655,10 +667,18 @@ static inline notrace int trace_event_get_offsets_#=
#call(		\
> #undef __string
> #define __string(item, src) __dynamic_array(char, item, -1)
>=20
> +#undef __string_len
> +#define __string_len(item, src, len) __dynamic_array(char, item, -1)
> +
> #undef __assign_str
> #define __assign_str(dst, src)						\
> 	strcpy(__get_str(dst), (src) ? (const char *)(src) : "(null)");
>=20
> +#undef __assign_str_len
> +#define __assign_str_len(dst, src, len)						\
> +	strncpy(__get_str(dst), (src) ? (const char *)(src) : "(null)", len);	\
> +	__get_str(dst)[len] =3D '\0';
> +
> #undef __bitmask
> #define __bitmask(item, nr_bits) __dynamic_array(unsigned long, item, -1)
>=20
>=20
>=20

--
Chuck Lever