[OpenWrt-Devel] [PATCH 2/4] tools: firmware-utils: add region code support to mktplinkfw

[Matthias Schiffer]

On 04/21/2016 11:28 AM, Rafał Miłecki wrote:
> On 21 April 2016 at 08:33, Matthias Schiffer
> <mschiffer at universe-factory.net> 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20160421/a90de8a9/attachment.sig>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel