This prevents hogging the CPU in a tight loop,
while waiting for access.
I've also reduced the number of tries to 30, rather
than 200. This is more conservative, while still
being somewhat permissive.
The addition of the usleep delay probably makes
this more reliable than the previous behaviour of
quickly spinning through 200 tries, but without
hogging CPU resources.
I *could* allow this loop to be infinite, but
I regard infinite spin-lock as an error state.
Signed-off-by: Leah Rowe <leah@libreboot.org>
a non-fatal error could have set errno. when we return
from check_read_or_die(), it should be assumed that
all is well.
i don't think this would mask anything important, but
it may be regarded as a preventative bug fix, since
it most likely only prevents false-positives.
Signed-off-by: Leah Rowe <leah@libreboot.org>
Part of the code currently assumes we only work on
the smaller NVM area.
I'm adding some comments to make this clear, for
when and if the code is ever expanded to support
operating on the Extended NVM area (just part the
main 128-byte NVM area).
Signed-off-by: Leah Rowe <leah@libreboot.org>
use of it was preventing more verbose error messages
on exit.
the code is actually cleaner without it, and easier
to read, because of those verbose error messages.
i also added some comments to cmd_swap/copy and did
some other minor/related cleanup elsewhere.
Signed-off-by: Leah Rowe <leah@libreboot.org>
split it out of main. this is good hygiene and it's preparation
for a planned expansion in the future, that allows operation
on multiple files.
Signed-off-by: Leah Rowe <leah@libreboot.org>
currently redundant, but again i might expand this
in the future to allow multiple runs. putting this
here as good practise (currently redundant).
Signed-off-by: Leah Rowe <leah@libreboot.org>
we currently only run the logic once, but i might
expand nvmutil in the future, so that it can
operate on multiple files. this would require
using a different command syntax, e.g. getop-style
syntax.
this is a preventative bug fix, resetting global
state.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we only ever use it once, so it's fine, but future
expansion of this code might trip us up.
this is therefore a preventative bug fix.
Signed-off-by: Leah Rowe <leah@libreboot.org>
size_t can truncate on some platforms. it's best to use
the proper variable type (a cast is insufficient).
Signed-off-by: Leah Rowe <leah@libreboot.org>
strtonum implementations in bsd sometimes have this
variable name. rename it to avoid conflict.
also removed the commentt errno values, since i'm
only ever setting it to valid values, as are the
syscalls that i'm using, so it should be ok.
i'm not writing a stub to check errno. that would
be far beyond the scope of nvmutil.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this is a bit of fault tolerance. a bit bloated too,
but it should make the code more resilient.
we limited the number of retries to 200 retries.
EINTR is when the syscall (read/pread) is interrupted.
we still error out on other conditions; we also still
error out on EINTR if the number of re-tries surpasses
200.
during this re-try loop, if *another* error occurs, we
exit as normal. this is done for both files: the gbe
file, and /dev/urandom.
Signed-off-by: Leah Rowe <leah@libreboot.org>
these functions return ssize_t, so compare explicitly
to that, when using the SIZE_4KB define for example.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we can just use errval as argument to set_err,
because set_err itself now properly handles
errno, ensuring that is is never set to zero.
Signed-off-by: Leah Rowe <leah@libreboot.org>
word/set_word are only meant to operate on the nvm
area (128 bytes), but the current check is against
the entire 4KB block.
swap() only handles the nvm area, as per the design
of nvmutil.
this patch makes the boundary check truer to my real
intent, guarding against future logical errors.
Signed-off-by: Leah Rowe <leah@libreboot.org>
alignment isn't an issue, but aliasing between uintX_t
types in C means that this code may fail on some weird
systems.
using memcpy here is advisable.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we don't need it. what follows is a call to open(), which
would fail anyway if the path is a directory; further, this
removes a theoretical race condition in the program, and
makes open() happen sooner, making it more likely that we
get the file first, before another program can take it.
checking whether /dev/urandom is a directory is the height
of absurdity.
Signed-off-by: Leah Rowe <leah@libreboot.org>
n + 1 is the same as saying sizeof(rnum) in this case.
we should be clear about that, in code. n is irrelevant
here, since it is only an index for the return value.
Signed-off-by: Leah Rowe <leah@libreboot.org>
checking against -1 is incorrect, because we specifically want
to ensure that it always read the number of bytes defined by
the size of rnum.
this still covers the case where the return value is -1, and
therefore makes the error handling much stricter.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the way err works here now is very different than
the bsd one. here, we ALWAYS exit with EXIT_FAILURE,
and we call set_err with, as argument, the first
argument given to err.
this then sets errno, but the exit value is always
consistent.
that's what happens when i control err(). i make it
even better. the original bsd one is too conservative.
Signed-off-by: Leah Rowe <leah@libreboot.org>
