qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>, Eric Blake <eblake@redhat.com>
Cc: QEMU Trivial <qemu-trivial@nongnu.org>,
	Peter Maydell <peter.maydell@linaro.org>,
	David CARLIER <devnexen@gmail.com>,
	qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [PATC 7/9] Skipping drm build, unsupported
Date: Wed, 1 Jul 2020 18:04:54 +0200	[thread overview]
Message-ID: <561e33c0-507b-ecc6-39ab-50b858eae315@redhat.com> (raw)
In-Reply-To: <20200701151524.wogl4ihbdevgf4bt@sirius.home.kraxel.org>

Hi Gerd,

On 7/1/20 5:15 PM, Gerd Hoffmann wrote:
>>> Ah, that is the problem.  Yes, DT_CHR is an non-posix optimization which
>>> allows to get the file type directly, without another round-trip to the
>>> kernel.  If that isn't available you can stat() the file and check
>>> ((st_mode & S_IFMT) == S_IFCHR) instead.
>>
>> Even when d_type and DT_CHR is available, there are filesystems where the
>> Linux kernel reports d_type of DT_UNKNOWN, and where you are best having
>> that code also falling back to an fstat().
> 
> Given this isn't perforance critical at all it is probably simplest to
> avoid non-portable d_type altogether and just to the fstat
> unconditionally.
> 
> David, does that work for haiku?
> 
> take care,
>   Gerd
> 
> diff --git a/util/drm.c b/util/drm.c
> index a23ff2453826..a1d3520d00f2 100644
> --- a/util/drm.c
> +++ b/util/drm.c
> @@ -24,6 +24,7 @@ int qemu_drm_rendernode_open(const char *rendernode)
>  {
>      DIR *dir;
>      struct dirent *e;
> +    struct stat st;
>      int r, fd;
>      char *p;
>  
> @@ -38,10 +39,6 @@ int qemu_drm_rendernode_open(const char *rendernode)
>  
>      fd = -1;
>      while ((e = readdir(dir))) {
> -        if (e->d_type != DT_CHR) {
> -            continue;
> -        }
> -
>          if (strncmp(e->d_name, "renderD", 7)) {
>              continue;
>          }
> @@ -53,6 +50,12 @@ int qemu_drm_rendernode_open(const char *rendernode)
>              g_free(p);
>              continue;
>          }
> +        fstat(r, &st);

While preparing the formal patch, can you add a comment here explaining
we deliberately use this way for portability (not checking DT_CHR /
DT_UNKNOWN ...)?

Thanks!

> +        if ((st.st_mode & S_IFMT) != S_IFCHR) {
> +            close(r);
> +            g_free(p);
> +            continue;
> +        }
>          fd = r;
>          g_free(p);
>          break;
> 



  parent reply	other threads:[~2020-07-01 16:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-29 21:48 [PATC 7/9] Skipping drm build, unsupported David CARLIER
2020-06-30  6:44 ` Philippe Mathieu-Daudé
2020-06-30  8:23   ` Gerd Hoffmann
2020-06-30  8:46     ` Philippe Mathieu-Daudé
2020-06-30 15:48       ` Gerd Hoffmann
2020-06-30 15:55         ` David CARLIER
2020-06-30 16:13           ` David CARLIER
2020-06-30 15:53     ` Peter Maydell
2020-06-30 16:53       ` Gerd Hoffmann
2020-07-01 14:16         ` Eric Blake
2020-07-01 15:15           ` Gerd Hoffmann
2020-07-01 15:48             ` David CARLIER
2020-07-01 16:04             ` Philippe Mathieu-Daudé [this message]
2020-07-01 16:53             ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=561e33c0-507b-ecc6-39ab-50b858eae315@redhat.com \
    --to=philmd@redhat.com \
    --cc=devnexen@gmail.com \
    --cc=eblake@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).