Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ENT-12033: Made multiple changes to increase robustness of remote file copying #5620

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

ENT-12033: Made multiple changes to increase robustness of remote file copying #5620

wants to merge 10 commits into from

Commits on Oct 29, 2024

  1. Added doc string to CopyRegularFileNet 

    Some of the parameters were not that obvious. Hence, I added a doc
string so that future hackers can save some time trying to understand
this madness.

Ticket: None
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
    @larsewi
    larsewi committed Oct 29, 2024
    Configuration menu
    Copy the full SHA
    23441a5 View commit details
    Browse the repository at this point in the history

  2. Added assert to check return code of snprintf 

    Added assert to check return code of snprintf in
`[Encrypt]CopyRegularFileNet`.

Ticket: None
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
    @larsewi
    larsewi committed Oct 29, 2024
    Configuration menu
    Copy the full SHA
    6a383db View commit details
    Browse the repository at this point in the history

  3. Fixed typo in comment for CopyRegularFileNet 

    Ticket: None
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
    @larsewi
    larsewi committed Oct 29, 2024
    Configuration menu
    Copy the full SHA
    7a905fd View commit details
    Browse the repository at this point in the history

  4. Removed check for filename too long 

    There was a check to see if the destination filename was larger than
`CF_BUFSIZE - 20` when copying files over the network. This makes no
sense to me.

At first glans, one might think that it has to do with the network
protocol. I.e., `GET $SIZE $FILENAME` has to fit in `CF_BUFSIZE`.
However, this is the source filename, and it is never sent over the
protocol, as far as I can see.

Tried to use git blame to see why it was introduced, but that was not of
much help. It appears to date back to the very first implmentation of
the files promise in 2008. I'll try removing it.

Ticket: None
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
    @larsewi
    larsewi committed Oct 29, 2024
    Configuration menu
    Copy the full SHA
    4726972 View commit details
    Browse the repository at this point in the history

  5. Removed unnecessary assignment of NULL-byte to buffer 

    Removed unnecessary assignment of terminating NULL-byte to buffer.
`snprintf` will terminate the buffer.

Ticket: None
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
    @larsewi
    larsewi committed Oct 29, 2024
    Configuration menu
    Copy the full SHA
    226f64d View commit details
    Browse the repository at this point in the history

  6. Check return code of sscanf 

    We should not rely on `sscanf` not mutating the pointer arguments on a
matching failure as this is not specified in the man page. Instead we
should check the expected amount of items are successfully matched.

Ticket: None
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
    @larsewi
    larsewi committed Oct 29, 2024
    Configuration menu
    Copy the full SHA
    d00d281 View commit details
    Browse the repository at this point in the history

  7. Added sanity check in CopyRegularFileNet 

    Added a sanity check to make sure we don't write more than the original
filesize aquired by SYNCH ... STAT source. This might happen if the file
was changed while copying.

Ticket: None
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
    @larsewi
    larsewi committed Oct 29, 2024
    Configuration menu
    Copy the full SHA
    8daa59b View commit details
    Browse the repository at this point in the history

  8. Allocate receive buffer on stack instead of heap 

    Changed to code in `[Encrypt]CopyRegularFileNet` to allocate the receive
buffer on the stack instead of the heap. There is no reason to have it
dynamically allocated since the size is constant. Furthermore, the
buffer is allocated and destroyed in the same scope.

Ticket: None
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
    @larsewi
    larsewi committed Oct 29, 2024
    Configuration menu
    Copy the full SHA
    dadf891 View commit details
    Browse the repository at this point in the history

  9. cf-serverd now stats file every read on network transmission 

    Previously the file was `stat`'ed every N read to detect file changes
during transmission. The reason for limiting the calls to stat is
probably to make the code more efficient. However, there are reasons to
believe that the bug experienced in ENT-12033 is attributed to
filechanges during transmission. Hence, I'm changing the code to do this
for every read, as it better to be safe and happy rather than fast but
sorry.

Yes, `stat()` is a system call. However, it has pretty good cashing
mechanisms on modern systems. So it should not be too expensive.

Ticket: None
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
    @larsewi
    larsewi committed Oct 29, 2024
    Configuration menu
    Copy the full SHA
    a026630 View commit details
    Browse the repository at this point in the history

  10. Added filestream flushing on error in EncryptCopyRegularFileNet 

    By flushing the filestream, we make sure that there is no junk data
after error.

Ticket: None
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
    @larsewi
    larsewi committed Oct 29, 2024
    Configuration menu
    Copy the full SHA
    68ffb07 View commit details
    Browse the repository at this point in the history