Re: Question on MODULE_VERSION macro

[Sam Ravnborg <sam@ravnborg.org>]

Hi Rusty.

I have not yet fully understood why you want to parse every source file.
I can see in the implemntation that you only calculate the sum of
code-lines, and not comments.
But why do we want to add this complexity - compared to just
calculating the sum of the whole file?
If the calculated sum is being presented as based on the source code
I assume people can understand that the sum does not match even after
updating a comment.

The current implementation fails to locate include files in the local
directory when compiled using "make O=...".
This is due to the fact that some files are present in the _deps
file with full path, others with relative path.

Example:
  /home/sam/bk/v2.6/drivers/scsi/aic7xxx/aiclib.h \
  /home/sam/bk/v2.6/drivers/scsi/aic7xxx/aic7xxx.h \
    $(wildcard include/config/used.h) \
  drivers/scsi/aic7xxx/aic7xxx_reg.h \
  /home/sam/bk/v2.6/drivers/scsi/aic7xxx/aic7xxx_inline.h \


I took a quick look and cannot explain why gcc spits out include files
with different paths.

My next question. Since we only parse a subset of the headers, is it
really needed to parse any of them?
My thinking is that we should either:
a) parse all header files (except those marked with $(wildcard))
b) parse no header files.

See also a few specific comments below.

	Sam

> +/* We have dir/file.o.  Open dir/.file.o.cmd, look for deps_ line to
> + * figure out source file. */
> +static int parse_source_files(const char *objfile, struct md4_ctx *md)
> +{
> +	char *cmd, *file, *p, *end;
> +	const char *base;
> +	unsigned long flen;
> +	int dirlen, ret = 0;
> +
> +	cmd = malloc(strlen(objfile) + sizeof("..cmd"));

You miss a "+ 1" to count for trailing '\0'.

> +
> +	base = strrchr(objfile, '/');
> +	if (base) {
> +		base++;
> +		dirlen = base - objfile;
> +		sprintf(cmd, "%.*s.%s.cmd", dirlen, objfile, base);
> +	} else {
> +		dirlen = 0;
> +		sprintf(cmd, ".%s.cmd", objfile);
> +	}
> +
> +	file = grab_file(cmd, &flen);
> +	if (!file) {
> +		fprintf(stderr, "Warning: could not find %s for %s\n",
> +			cmd, objfile);
> +		goto out;
> +	}
> +
> +	/* There will be a line like so:
> +		deps_drivers/net/dummy.o := \
> +		  drivers/net/dummy.c \
> +		    $(wildcard include/config/net/fastroute.h) \
> +		  include/linux/config.h \
> +		    $(wildcard include/config/h.h) \
> +		  include/linux/module.h \
> +
> +	   Sum all files in the same dir or subdirs.
> +	*/
> +	/* Strictly illegal: file is not nul terminated. */
> +	p = strstr(file, "\ndeps_");
> +	if (!p) {
> +		fprintf(stderr, "Warning: could not find deps_ line in %s\n",
> +			cmd);
> +		goto out_file;
> +	}
> +	p = strstr(p, ":=");
> +	if (!p) {
> +		fprintf(stderr, "Warning: could not find := line in %s\n",
> +			cmd);
> +		goto out_file;
> +	}
> +	p += strlen(":=");
> +	p += strspn(p, " \\\n");
> +
> +	end = strstr(p, "\n\n");
> +	if (!end) {
> +		fprintf(stderr, "Warning: could not find end line in %s\n",
> +			cmd);
> +		goto out_file;
> +	}
> +
> +	while (p < end) {
> +		unsigned int len;
> +
> +		len = strcspn(p, " \\\n");
> +		if (memcmp(objfile, p, dirlen) == 0) {
> +			char source[len + 1];
gcc extension, you do not want to use malloc here?

> +
> +			memcpy(source, p, len);
> +			source[len] = '\0';
> +			printf("parsing %s\n", source);
Debug printf - to be deleted.

> +			if (!parse_file(source, md)) {
> +				fprintf(stderr,
> +					"Warning: could not open %s: %s\n",
> +					source, strerror(errno));
> +				goto out_file;
> +			}
> +		}
> +		p += len;
> +		p += strspn(p, " \\\n");
> +	}
> +
> +	/* Everyone parsed OK */
> +	ret = 1;
> +out_file:
> +	release_file(file, flen);
> +out:
> +	free(cmd);
> +	return ret;
> +}