This code has at least a couple of problems.

- using atol is considered a Bad Thing.  One reason is that errors
  cannot be caught.  Use strtol instead.

- atol does not set errno, but the code assumes that it does.

- at least once, errno is set to 0 after the atol call and then the
  value is tested as if it might have been set by atol.  Doubly
  impossible

   169                          p_priority_offset = -atol($3.buf);
   170
   171                          errno = 0;
   172                          if (errno != 0 || p_priority_offset < INT32_MIN)

- the result of an atol call might be the most negative number and
  hence negating it might cause Undefined Behavior.

- p_priority_offset is a file static variable and yet each use is
  local to a block.  It would be better as a local auto.

The most recent log entry suggests that we are downsteam of NetBSD so
perhaps any fixes should be done there.

This does not suggest good quality code.

Do we care enough about the code to improve it?

If so, should we do it through NetBSD?  Is anyone here engaged with
NetBSD developers?

commit d3a105e52f7aec6f58aa8b6bcff5f3431ca57394
Author: Andrew Cagney <[email protected]>
Date:   Tue Jun 2 11:30:26 2020 -0400

    bsd: suck in NetBSD's 2014 version of libpfkey
    
    It's less embedded than the FreeBSD version; known to break BSD
    builds.



_______________________________________________
Swan-dev mailing list
[email protected]
https://lists.libreswan.org/mailman/listinfo/swan-dev

Reply via email to