On 04/21/2016 11:28 AM, Rafał Miłecki wrote: > On 21 April 2016 at 08:33, Matthias Schiffer > <[email protected]> wrote: >> @@ -324,6 +351,17 @@ static int check_options(void) >> else >> hw_rev = 1; >> >> + if (country) { >> + region = find_region(country); >> + if (region == (uint32_t)-1) { >> + char *end; >> + region = strtoul(country, &end, 0); >> + if (*end) { >> + ERR("unknown region code \"%s\"", country); >> + } >> + } >> + } > > I'm wondering if this wouldn't be better to just make find_region > return signed int. > > What about failing to build firmware if a provided country can't be > converted into an unsigned int? Right now you set "region" field in > the header to ULONG_MAX if country is invalid. This tool usually > refuses to build firmware if something goes wrong, e.g.: > > ERR("either board or hardware id must be specified"); > return -1; > > ERR("unknown/unsupported board id \"%s\"", board_id); > return -1; > > ERR("flash layout is not specified"); > return -1; >
I don't think returning signed makes sense, uint32_t is also used in the struct (and everywhere else). Regarding the error case: Yes, I forgot the 'return -1' there, I'll send a v2. Matthias
signature.asc
Description: OpenPGP digital signature
_______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
