Sorry about the formatting error. Not sure how I managed that.
I will get a version fixing these issues to you shortly.
Thanks,
Alfie
On 16/10/2025 18:10, Wilco Dijkstra wrote:
Hi Alfie,
A few more comments below - the digit parsing looks fine now.
Note tabs appear missing so it's impossible to patch the diff as emailed -
can you make sure the actual patch has correct tabs please?
- while (str.is_valid ())
+ /* Parse the architecture part of the string. */
+ string_slice arch_string = string_slice::tokenize (&str, ";");
+ while (arch_string.is_valid ())
{
string_slice ext;
- ext = string_slice::tokenize (&str, "+");
+ ext = string_slice::tokenize (&arch_string, "+");
Add a .strip() at the end for consistency with the rest of the parser.
gcc_assert (ext.is_valid ());
Note this assert is redundant too, and so is the !ext.is_valid () part in
the next if-statement.
+ unsigned long priority_res = 0;
Why use unsigned long when *priority is unsigned int?
+ string_slice argument = string_slice::tokenize (&str, ";");
+ argument = argument.strip ();
Why not combine the strip on the same line like below?
+ string_slice arg = argument;
+ string_slice name = string_slice::tokenize (&argument, "=").strip ();
+ if (!argument.is_valid () || argument.empty ())
+ {
+ *invalid_extension = std::string (arg.begin (), arg.size ());
+ return AARCH_PARSE_INVALID_FEATURE;
+ }
Missing an argument.strip() somewhere here? (and then argument.empty ()
above is redundant).
+ for (char c : argument)
+ {
+ if (!ISDIGIT (c))
+ {
+ priority_res = 0;
+ break;
+ }
+ priority_res = 10 * priority_res + c - '0';
Indentation looks wrong.
+ }
+
+ /* Check the entire string parsed, and that the value is in range. */
+ if (priority_res < 1
+ || priority_res > 255)
Merge onto single line.
Cheers,
Wilco
