On 08/29/2017 06:02 PM, Bart Van Assche wrote: > On Tue, 2017-08-29 at 10:49 +0200, Hannes Reinecke wrote: >> [ ... ] >> +$(obj)/scsi_sysfs.o: $(obj)/scsi_devinfo_tbl.c >> + >> +quiet_cmd_bflags = GEN $@ >> + cmd_bflags = $(PERL) -s $(src)/mktbl.pl BLIST $< > $@ > > Hello Hannes, > > Is it considered acceptable to require that perl is available to build the > kernel? People who build the kernel for embedded systems probably won't be > happy if this is a new requirement. See e.g. "PATCH [0/3]: Simplify the > kernel build by removing perl" (https://lkml.org/lkml/2009/1/2/20). > > Have you noticed that for other generated files a _shipped version is > provided? > Ah. Good point. Will be doing so. >> + >> +$(obj)/scsi_devinfo_tbl.c: include/scsi/scsi_devinfo.h >> + $(call if_changed,bflags) >> + >> # If you want to play with the firmware, uncomment >> # GENERATE_FIRMWARE := 1 >> >> diff --git a/drivers/scsi/mktbl.pl b/drivers/scsi/mktbl.pl >> [ ... ] >> +my $prf; >> + >> +$prf = $ARGV[0]; >> +open IN_FILE, "<$ARGV[1]" || die; >> +print "\t/*\n\t * Automatically generated by ", $0, ".\n"; >> +print "\t * Do not edit.\n\t */\n"; >> +while (<IN_FILE>) { >> + chomp; >> + if (/^#define ${prf}_([^[:blank:]]*).*/) { >> + print "\t{ ", $prf, "_", $1, ", \"", $1, "\" },\n"; >> + } >> +} >> +close IN_FILE || die; > > Can this be done with sed? Do we really need Perl for this? > In principle, sure. I just failed to stuff everything into a makefile line, so perl was easier to code. >> [ ... ] >> +static const struct { >> + unsigned int value; >> + char *name; > > Can 'char *' be changed into 'const char *'? > Sure; why not. >> +} sdev_bflags[] = { >> +#include "scsi_devinfo_tbl.c" >> +}; >> + >> +static const char *sdev_bflags_name(unsigned int bflags) >> +{ >> + int i; >> + const char *name = NULL; >> + >> + for (i = 0; i < ARRAY_SIZE(sdev_bflags); i++) { >> + if (sdev_bflags[i].value == bflags) { >> + name = sdev_bflags[i].name; >> + break; >> + } >> + } >> + return name; >> +} > > How about using ilog2() of the BLIST_* values as index of the table such that > an array lookup can be used instead of a for-loop over the entire array? > Not sure if that'll work. The BLIST_* values in fact form a sparse array, so the lookup array 'sdev_bflags' actually an associative array. And I dare not convert it to a normal array as then I couldn't to the automatice table creation anymore. So I'm not sure if ilog2 will save me anything here. >> + >> static int check_set(unsigned long long *val, char *src) >> { >> char *last; >> @@ -955,6 +977,43 @@ static DEVICE_ATTR(queue_depth, S_IRUGO | S_IWUSR, sdev_show_queue_depth, >> } >> static DEVICE_ATTR(wwid, S_IRUGO, sdev_show_wwid, NULL); >> >> +static ssize_t >> +sdev_show_blacklist(struct device *dev, struct device_attribute *attr, >> + char *buf) >> +{ >> + struct scsi_device *sdev = to_scsi_device(dev); >> + int i; >> + char *ptr = buf; >> + ssize_t len = 0; >> + >> + for (i = 0; i < sizeof(unsigned int) * 8; i++) { >> + unsigned int bflags = (unsigned int)1 << i; > > How about using 1U instead of (unsigned int)1? > Yeah, ok. >> + ssize_t blen; >> + const char *name = NULL; >> + >> + if (!(bflags & sdev->sdev_bflags)) >> + continue; >> + >> + if (ptr != buf) { >> + blen = snprintf(ptr, 2, " "); >> + ptr += blen; >> + len += blen; >> + } > > Should this code check whether or not it overflows the output buffer @buf? > Hmm. In principle, yes. However, as we're only have a limited number of possible arguments and hence a fixed upper limit of what we can print into the buffer I'm not sure if that's required. But I'll check. >> + name = sdev_bflags_name(bflags); >> + if (name) >> + blen = snprintf(ptr, strlen(name) + 1, >> + "%s", name); >> + else >> + blen = snprintf(ptr, 67, "INVALID_BIT(%d)", i); >> + ptr += blen; >> + len += blen; >> + } > > Should this code check too whether or not it overflows the output buffer @buf? > See above. Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@xxxxxxx +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
