load_file_data: do not close fd on error to avoid double-close

[cgzones]

load_file_at() is affected in the error case, since all other current callers call die_with_error() before closing the passed file descriptor.

Found by GCC analyzer:

./utils.c: In function ‘load_file_at’:
./utils.c:630:3: warning: double ‘close’ of file descriptor ‘fd’ [CWE-1341] [-Wanalyzer-fd-double-close]
630 |   close (fd);
    |   ^~~~~~~~~~
‘load_file_at’: events 1-4
    |
    |  616 | load_file_at (int         dirfd,
    |      | ^~~~~~~~~~~~
    |      | |
    |      | (1) entry to ‘load_file_at’
    |......
    |  624 |   if (fd == -1)
    |      |      ~
    |      |      |
    |      |      (2) following ‘false’ branch (when ‘fd != -1’)...
    |......
    |  627 |   data = load_file_data (fd, NULL);
    |      |   ~~~~   ~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |   |      |
    |      |   |      (4) calling ‘load_file_data’ from ‘load_file_at’
    |      |   (3) ...to here
    |
    +--> ‘load_file_data’: events 5-6
        |
        |  568 | load_file_data (int     fd,
        |      | ^~~~~~~~~~~~~~
        |      | |
        |      | (5) entry to ‘load_file_data’
        |......
        |  579 |   data = xmalloc (data_len);
        |      |          ~~~~~~~~~~~~~~~~~~
        |      |          |
        |      |          (6) calling ‘xmalloc’ from ‘load_file_data’
        |
        +--> ‘xmalloc’: events 7-9
                |
                |  119 | xmalloc (size_t size)
                |      | ^~~~~~~
                |      | |
                |      | (7) entry to ‘xmalloc’
                |......
                |  123 |   if (res == NULL)
                |      |      ~
                |      |      |
                |      |      (8) following ‘false’ branch (when ‘res’ is non-NULL)...
                |  124 |     die_oom ();
                |  125 |   return res;
                |      |   ~~~~~~
                |      |   |
                |      |   (9) ...to here
                |
        <------+
        |
        ‘load_file_data’: events 10-11
        |
        |  579 |   data = xmalloc (data_len);
        |      |          ^~~~~~~~~~~~~~~~~~
        |      |          |
        |      |          (10) returning to ‘load_file_data’ from ‘xmalloc’
        |......
        |  583 |       if (data_len == data_read + 1)
        |      |          ~
        |      |          |
        |      |          (11) following ‘false’ branch...
        |
        ‘load_file_data’: event 12
        |
        |cc1:
        | (12): ...to here
        |
        ‘load_file_data’: events 13-16
        |
        |  571 |   cleanup_free char *data = NULL;
        |      |                      ~
        |      |                      |
        |      |                      (16) inlined call to ‘cleanup_freep’ from ‘load_file_data’
        |......
        |  591 |       while (res < 0 && errno == EINTR);
        |      |                      ^
        |      |                      |
        |      |                      (13) following ‘false’ branch...
        |  592 |
        |  593 |       if (res < 0)
        |      |       ~~
        |      |       |
        |      |       (14) ...to here
        |......
        |  596 |           close (fd);
        |      |           ~~~~~~~~~~
        |      |           |
        |      |           (15) first ‘close’ here
        |
        +--> ‘cleanup_freep’: events 17-18
                |
                |./utils.h:141:6:
                |  141 |   if (*pp)
                |      |      ^
                |      |      |
                |      |      (17) following ‘true’ branch (when ‘data’ is non-NULL)...
                |  142 |     free (*pp);
                |      |     ~~~~
                |      |     |
                |      |     (18) ...to here
                |
    <-------------+
    |
‘load_file_at’: events 19-20
    |
    |./utils.c:627:10:
    |  627 |   data = load_file_data (fd, NULL);
    |      |          ^~~~~~~~~~~~~~~~~~~~~~~~~
    |      |          |
    |      |          (19) returning to ‘load_file_at’ from ‘load_file_data’
    |......
    |  630 |   close (fd);
    |      |   ~~~~~~~~~~
    |      |   |
    |      |   (20) second ‘close’ here; first ‘close’ was at (15)

[smcv]

This doesn't seem like a practical problem: we don't open any other fds between the first close() and the second, and bubblewrap is single-threaded (so there is no other thread opening fds), so the second close() will deterministically fail with EBADF rather than closing an unintended fd.

[smcv]

Please could you update the commit message to mention that this isn't a practical problem, and not include several pages of gcc output?