CV-Bowen commented on code in PR #21373: URL: https://github.com/apache/nuttx/pull/21373#discussion_r4450561766
########## drivers/rpmsg/rpmsg_procfs.c: ########## @@ -0,0 +1,271 @@ +/**************************************************************************** + * drivers/rpmsg/rpmsg_procfs.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <sys/types.h> +#include <sys/stat.h> + +#include <assert.h> +#include <errno.h> +#include <debug.h> +#include <fnmatch.h> + +#include <nuttx/fs/fs.h> +#include <nuttx/fs/procfs.h> +#include <nuttx/kmalloc.h> +#include <nuttx/nuttx.h> +#include <nuttx/rpmsg/rpmsg.h> + +#include "rpmsg.h" +#include "rpmsg_procfs.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/* Determines the size of an intermediate buffer that must be large enough + * to handle the longest line generated by this logic. + */ + +#define RPMSG_PROCFS_LINELEN 80 + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +/* This structure describes one open "file" */ + +struct rpmsg_procfs_file_s +{ + struct procfs_file_s base; /* Base open file structure */ + char line[RPMSG_PROCFS_LINELEN]; /* Pre-allocated buffer for formatted lines */ +}; + +struct rpmsg_procfs_info_s +{ + FAR struct rpmsg_procfs_file_s *procfile; + size_t totalsize; + FAR char *buffer; + size_t buflen; + off_t offset; +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +/* File system methods */ + +static int rpmsg_procfs_open(FAR struct file *filep, + FAR const char *relpath, + int oflags, mode_t mode); +static int rpmsg_procfs_close(FAR struct file *filep); +static ssize_t rpmsg_procfs_read(FAR struct file *filep, FAR char *buffer, + size_t buflen); +static ssize_t rpmsg_procfs_write(FAR struct file *filep, + FAR const char *buffer, size_t buflen); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static const struct procfs_operations g_rpmsg_procfs_ops = Review Comment: Fixed: added rpmsg_procfs_dup() and rpmsg_procfs_stat(). `ls -l /proc` used to assert at fs_procfs.c:1169 and now lists `-r--r--r-- rpmsg` (`-rw-r--r--` once the filter commit adds write). ########## drivers/rpmsg/rpmsg_trace.c: ########## @@ -0,0 +1,127 @@ +/**************************************************************************** + * drivers/rpmsg/rpmsg_trace.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <stdarg.h> +#include <stdint.h> +#include <sys/param.h> +#include <stdbool.h> +#include <execinfo.h> + +#include <nuttx/sched_note.h> +#include <nuttx/spinlock.h> +#include <nuttx/streams.h> +#include <nuttx/rpmsg/rpmsg.h> + +#include "rpmsg_procfs.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/* nc_length is a uint8_t, so a payload has to be split into notes that + * each fit in it. + */ + +#define RPMSG_TRACE_CHUNK (UINT8_MAX - SIZEOF_NOTE_EVENT(0)) + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +/* Keeps a text record and its binary payload next to each other in the + * note stream when several CPUs trace at the same time. + */ + +static spinlock_t g_rpmsg_trace_lock = SP_UNLOCKED; + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: rpmsg_trace + ****************************************************************************/ + +void rpmsg_trace(FAR const char *name, bool bt, FAR const void *buf, + size_t len, FAR const char *format, ...) +{ + va_list ap; + + va_start(ap, format); + rpmsg_vtrace(name, bt, buf, len, format, ap); + va_end(ap); +} + +/**************************************************************************** + * Name: rpmsg_vtrace + ****************************************************************************/ + +void rpmsg_vtrace(FAR const char *name, bool bt, FAR const void *buf, + size_t len, FAR const char *format, va_list ap) +{ + irqstate_t flags; + + if (!rpmsg_procfs_trace_allow(name)) + { + return; + } + +#if CONFIG_RPMSG_TRACE_BACKTRACE > 0 + if (bt) + { + FAR void *frames[CONFIG_RPMSG_TRACE_BACKTRACE]; + char btbuf[BACKTRACE_BUFFER_SIZE(CONFIG_RPMSG_TRACE_BACKTRACE)]; + struct va_format vaf; + + vaf.fmt = format; + vaf.va = ≈ Review Comment: Fixed: both paths now va_copy() ap into a local va_list. The x86_64 sim build with RPMSG_TRACE has no warning left in rpmsg_trace.c; before it had the two incompatible-pointer warnings. ########## drivers/rpmsg/rpmsg_trace.c: ########## @@ -0,0 +1,127 @@ +/**************************************************************************** + * drivers/rpmsg/rpmsg_trace.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <stdarg.h> +#include <stdint.h> +#include <sys/param.h> +#include <stdbool.h> +#include <execinfo.h> + +#include <nuttx/sched_note.h> +#include <nuttx/spinlock.h> +#include <nuttx/streams.h> +#include <nuttx/rpmsg/rpmsg.h> + +#include "rpmsg_procfs.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/* nc_length is a uint8_t, so a payload has to be split into notes that + * each fit in it. + */ + +#define RPMSG_TRACE_CHUNK (UINT8_MAX - SIZEOF_NOTE_EVENT(0)) + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +/* Keeps a text record and its binary payload next to each other in the + * note stream when several CPUs trace at the same time. + */ + +static spinlock_t g_rpmsg_trace_lock = SP_UNLOCKED; + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: rpmsg_trace + ****************************************************************************/ + +void rpmsg_trace(FAR const char *name, bool bt, FAR const void *buf, + size_t len, FAR const char *format, ...) +{ + va_list ap; + + va_start(ap, format); + rpmsg_vtrace(name, bt, buf, len, format, ap); + va_end(ap); +} + +/**************************************************************************** + * Name: rpmsg_vtrace + ****************************************************************************/ + +void rpmsg_vtrace(FAR const char *name, bool bt, FAR const void *buf, + size_t len, FAR const char *format, va_list ap) +{ + irqstate_t flags; + + if (!rpmsg_procfs_trace_allow(name)) + { + return; + } + +#if CONFIG_RPMSG_TRACE_BACKTRACE > 0 + if (bt) + { + FAR void *frames[CONFIG_RPMSG_TRACE_BACKTRACE]; + char btbuf[BACKTRACE_BUFFER_SIZE(CONFIG_RPMSG_TRACE_BACKTRACE)]; + struct va_format vaf; + + vaf.fmt = format; + vaf.va = ≈ + + backtrace(frames, CONFIG_RPMSG_TRACE_BACKTRACE); + backtrace_format(btbuf, sizeof(btbuf), frames, + CONFIG_RPMSG_TRACE_BACKTRACE); + flags = spin_lock_irqsave(&g_rpmsg_trace_lock); + sched_note_printf(NOTE_TAG_RPMSG, "%pV, %s", &vaf, btbuf); Review Comment: Fixed: the message is formatted with vsnprintf() into a 256-byte local buffer (one note printf record), the backtrace is appended, and the record is "%s". `trace dump` on qemu now shows `[Port] rx ept->cb start ept:0x404cc0d8 name:rpmsg-ping ... len:80, 0x0000000040307f24 ...`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
