5:31 p.m.
On 04/21/2014 04:05 PM, Eric Blake wrote:
On 04/20/2014 05:53 AM, Natanael Copa wrote:
> Introduce a wrapper for readdir. This helps us make sure that we always
> set errno before calling readdir and it will make sure errors are
> properly logged.
>
There's also the point I raised about whether we should make the
wrapper
function automatically skip . and ..; this would best be done by adding
a flags argument, where the default of 0 returns all entries, but
passing a non-zero flag can do filtering. I guess it still remains to
be seen how many callers would benefit from this. Also, your series
only touched one file to use the new paradigm; but ideally we'd add a
syntax check that enforces its use everywhere.
Something like this will make it easy to pick out the remaining spots in
the code that need conversion, and enforce the style in the future. You
can apply it now to find all the culprits, but rebase it to be the last
patch in the series to avoid bisection failures.
========
diff --git i/cfg.mk w/cfg.mk
index 8a444d6..3f4bba0 100644
--- i/cfg.mk
+++ w/cfg.mk
@@ -421,6 +421,12 @@ sc_prohibit_gethostname:
halt='use virGetHostname, not gethostname' \
$(_sc_search_regexp)
+sc_prohibit_readdir:
+ @prohibit='\breaddir *\(' \
+ exclude='exempt from syntax-check' \
+ halt='use virDirRead, not readdir' \
+ $(_sc_search_regexp)
+
sc_prohibit_gettext_noop:
@prohibit='gettext_noop *\(' \
halt='use N_, not gettext_noop' \
diff --git i/src/util/virfile.c w/src/util/virfile.c
index b54b9fd..54a1b25 100644
--- i/src/util/virfile.c
+++ w/src/util/virfile.c
@@ -2299,7 +2299,7 @@ virDirCreate(const char *path ATTRIBUTE_UNUSED,
int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname)
{
errno = 0;
- *ent = readdir(dirp);
+ *ent = readdir(dirp); /* exempt from syntax-check */
if (!*ent && errno) {
if (dirname)
virReportSystemError(errno, _("Unable to read directory
'%s'"),
=========
For reference, here's what still needs cleaning up (I ran this before
applying your patch 2/2, so the three nodeinfo.c instances go away with
that one). Hmm, the secret_driver.c points out a false positive in the
syntax check, where we'd have to tweak the comment wording.
src/conf/domain_conf.c:18136: while ((entry = readdir(dir))) {
src/conf/network_conf.c:3151: while ((entry = readdir(dir))) {
src/conf/network_conf.c:3185: while ((entry = readdir(dir))) {
src/conf/nwfilter_conf.c:3108: while ((entry = readdir(dir))) {
src/conf/storage_conf.c:1874: while ((entry = readdir(dir))) {
src/nodeinfo.c:456: while ((cpudirent = readdir(cpudir))) {
src/nodeinfo.c:494: while ((cpudirent = readdir(cpudir))) {
src/nodeinfo.c:676: while ((nodedirent = readdir(nodedir))) {
src/openvz/openvz_conf.c:1109: while ((dent = readdir(dp))) {
src/parallels/parallels_storage.c:104: while ((ent = readdir(dir)) !=
NULL) {
src/parallels/parallels_storage.c:342: while ((ent = readdir(dir)) !=
NULL) {
src/qemu/qemu_driver.c:442: while ((entry = readdir(dir))) {
src/qemu/qemu_hostdev.c:98: while ((iommuGroup = readdir(iommuDir))) {
src/secret/secret_driver.c:487: while ((de = readdir(dir)) != NULL) {
src/secret/secret_driver.c:506: /* Ignore error reported by
readdir(), if any. It's better to keep the
src/storage/storage_backend.c:1610: while ((dent = readdir(dh)) !=
NULL) {
src/storage/storage_backend_fs.c:865: while ((ent = readdir(dir)) !=
NULL) {
src/storage/storage_backend_iscsi.c:110: while ((dirent =
readdir(sysdir))) {
src/storage/storage_backend_scsi.c:255: while ((block_dirent =
readdir(block_dir))) {
src/storage/storage_backend_scsi.c:336: while ((lun_dirent =
readdir(lun_dir))) {
src/storage/storage_backend_scsi.c:445: while ((lun_dirent =
readdir(devicedir))) {
src/util/vircgroup.c:3154: ent = readdir(grpdir);
src/util/vircgroup.c:3399: while ((ent = readdir(dp))) {
src/util/vircgroup.c:3724: while ((de = readdir(dh)) != NULL) {
src/util/virfile.c:611: while ((de = readdir(dh)) != NULL) {
src/util/virfile.c:784: while ((de = readdir(dh)) != NULL) {
src/util/virfile.c:920: while ((de = readdir(dh)) != NULL) {
src/util/virnetdevtap.c:108: while ((dp = readdir(dirp)) != NULL) {
src/util/virpci.c:448: while ((entry = readdir(dir))) {
src/util/virpci.c:1921: while ((ent = readdir(dir)) != NULL) {
src/util/virpci.c:1976: while ((errno = 0, ent = readdir(groupDir))
!= NULL) {
src/util/virpci.c:2641: while ((entry = readdir(dir))) {
src/util/virscsi.c:136: while ((entry = readdir(dir))) {
src/util/virscsi.c:181: while ((entry = readdir(dir))) {
src/util/virusb.c:149: while ((de = readdir(dir))) {
src/util/virutil.c:1870: while ((entry = readdir(dir))) {
src/util/virutil.c:1952: while ((entry = readdir(dir))) {
src/xen/xen_inotify.c:366: while ((ent = readdir(dh))) {
src/xen/xm_internal.c:330: while ((ent = readdir(dh))) {
maint.mk: use virDirRead, not readdir
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org