Elizabeth Figura (@zfigura) commented about dlls/ntdll/unix/socket.c:
status = STATUS_INVALID_PARAMETER;
while ((c = fgetc(f)) != EOF)
{
if (c != '\n' || i++ != in_data->adapternum)
{
continue;
}
c = fscanf(f, "%02hhX%02hhX%02hhX%02hhX %02hhX%02hhX%02hhX%02hhX%02hhX%02hhX",
out_data->netnum, out_data->netnum+1, out_data->netnum+2, out_data->netnum+3,
out_data->nodenum, out_data->nodenum+1, out_data->nodenum+2,
out_data->nodenum+3, out_data->nodenum+4, out_data->nodenum+5);
status = (c < 10) ? STATUS_INVALID_ADDRESS : STATUS_SUCCESS;
break;
}
fclose(f);
if (status != STATUS_SUCCESS) break;
The way this loop is written is compact, but doesn't seem very idiomatic or readable. The reuse of "c" for two different purposes doesn't help matters either. I'd advocate for something like
```c for (;;) { /* nb this skips the first line, which is a header */ while ((c = fgetc(f)) != EOF && c != '\n') ;
ret = fscanf(f, ...); if (ret == EOF) { status = STATUS_INVALID_PARAMETER; break; } else if (ret != 10) { /* it may be nice to ERR() with the failing line... */ status = STATUS_INVALID_ADDRESS; break; }
if (i++ == count) { status = STATUS_SUCCESS; break; } } ```
I'd also personally argue for a helper instead of using that trampoline at the end.