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-ci...@xs4all.nl>
>> ---
>> 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
