On 2/15/19 10:22 AM, Laurent Pinchart wrote: > Hi Hans, > > Thank you for the patch. > > On Fri, Feb 15, 2019 at 09:10:24AM +0100, Hans Verkuil wrote: >> If the device passed to the -d option is not found, then interpret it >> as a bus-info string and try to open all media devices and see which one >> reports a bus-info string equal to the -d argument. >> >> That makes it possible to open a specific media device without having to know >> the name of the media device. >> >> Similar functionality has been implemented for v4l2-ctl and v4l2-compliance, >> and for the cec utilities. >> >> This allows scripts that no longer need to care about the name of a device >> node, instead they can find it based on a unique string. >> >> Also extend the -d option to support -d0 as a shorthand for /dev/media0 to >> make it consistent with the other utils. >> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> >> --- >> Changes since v1: >> - fold into the -d option instead of creating a separate option >> --- >> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c >> index 16367857..fb923775 100644 >> --- a/utils/media-ctl/options.c >> +++ b/utils/media-ctl/options.c >> @@ -19,13 +19,18 @@ >> * along with this program. If not, see <http://www.gnu.org/licenses/>. >> */ >> >> +#include <ctype.h> >> +#include <dirent.h> >> +#include <fcntl.h> >> #include <getopt.h> >> #include <stdio.h> >> #include <stdlib.h> >> #include <string.h> >> +#include <sys/ioctl.h> >> #include <unistd.h> >> #include <v4l2subdev.h> >> >> +#include <linux/media.h> >> #include <linux/videodev2.h> >> >> #include "options.h" >> @@ -43,6 +48,9 @@ static void usage(const char *argv0) >> >> printf("%s [options]\n", argv0); >> printf("-d, --device dev Media device name (default: %s)\n", MEDIA_DEVNAME_DEFAULT); >> + printf(" If <dev> starts with a digit, then /dev/media<dev> is used.\n"); >> + printf(" If <dev> doesn't exist, then find a media device that\n"); >> + printf(" reports a bus info string equal to <dev>.\n"); >> printf("-e, --entity name Print the device name associated with the given entity\n"); >> printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to setup\n"); >> printf(" --get-v4l2 pad Print the active format on a given pad\n"); >> @@ -161,6 +169,48 @@ static void list_known_mbus_formats(void) >> } >> } >> >> +static const char *make_devname(const char *device) >> +{ >> + static char newdev[300]; > > 300 is still a lot for such a short string. > >> + struct dirent *ep; >> + DIR *dp; >> + >> + if (!access(device, F_OK)) >> + return device; >> + >> + if (device[0] >= '0' && device[0] <= '9' && strlen(device) <= 3) { >> + snprintf(newdev, sizeof(newdev), "/dev/media%s", device); >> + return newdev; It's short for this, >> + } >> + >> + dp = opendir("/dev"); >> + if (dp == NULL) >> + return device; >> + >> + while ((ep = readdir(dp))) { >> + const char *name = ep->d_name; >> + >> + if (!memcmp(name, "media", 5) && isdigit(name[5])) { >> + struct media_device_info mdi; >> + int ret; >> + int fd; >> + >> + snprintf(newdev, sizeof(newdev), "/dev/%s", name); but not for this. ep->d_name can be up to 255 chars. I can use snprintf, but then gcc complains that the string will be truncated. >> + fd = open(newdev, O_RDWR); > > Did openat() fail ? There is no point, if this is a media device matching the bus_info, then I need to return newdev anyway. > >> + if (fd < 0) >> + continue; >> + ret = ioctl(fd, MEDIA_IOC_DEVICE_INFO, &mdi); >> + close(fd); >> + if (!ret && !strcmp(device, mdi.bus_info)) { >> + closedir(dp); >> + return newdev; >> + } >> + } >> + } >> + closedir(dp); >> + return device; >> +} >> + >> int parse_cmdline(int argc, char **argv) >> { >> int opt; >> @@ -175,7 +225,7 @@ int parse_cmdline(int argc, char **argv) >> opts, NULL)) != -1) { >> switch (opt) { >> case 'd': >> - media_opts.devname = optarg; >> + media_opts.devname = make_devname(optarg); >> break; >> >> case 'e': > Regards, Hans
