On 04/26/2016 11:28 PM, Dhole wrote:
I've fixed all the spaces issues. I've also changed the "unsigned long long" to "time_t" as you suggested. Also, to improve reliability I'm now using strtoll rather than strtoull, so that negative values can be detected in SOURCE_DATE_EPOCH, which are treated as errors. This way the variable pfile->source_date_epoch can't be set to -1 (which is the default value) when SOURCE_DATE_EPOCH is defined.
Some minor issues remain, but then I think it'll be good to go.
+ epoch = strtoll (source_date_epoch, &endptr, 10); + if ((errno == ERANGE && (epoch == LLONG_MAX || epoch == LLONG_MIN)) + || (errno != 0 && epoch == 0)) + fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: " + "strtoll: %s\n", xstrerror(errno)); + if (endptr == source_date_epoch) + fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: " + "No digits were found: %s\n", endptr); + if (*endptr != '\0') + fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: " + "Trailing garbage: %s\n", endptr); + if (epoch < 0) + fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: " + "Value must be nonnegative: %lld \n", epoch);
These are somewhat unusual for error messages, but I think the general principle of no capitalization probably applies, so "No", "Trailing", and "Value" should be lowercase.
Not sure whether these should be fatal_errors or warnings. For now it's probably ok to leave it as-is. Also, I wonder whether zero might be a better value to use as an unset marker, but again it's probably fine for now.
+ time_t source_date_epoch = (time_t) -1; + + source_date_epoch = get_source_date_epoch ();
First initialization seems unnecessary. Might want to merge the declaration with the initialization.
Ok with these changes. Bernd
