Fixed inability to install a larger/smaller ticket when a smaller/large one is installed

[ZeroSkill1]

If you have a ticket installed that is smaller or larger than the ticket you're trying to overwrite it with, the process will fail. Realistically this won't happen but if it does this should handle it.

This involves deleting the installed ticket, then installing the one the user wants to install, separately.

[aspargas2]

Overall I think this is a probably a good idea to have as a feature, but I think there's a bug in this implementation: consider the case that replace is passed as false to AddTicketToDB, and there is an existing ticket in the filesystem with the passed title ID. The expected behavior would then be for the function to change nothing in the ticket.db and return failure. This code, if I am reading it correctly, will always make AddBDRIEntry return 2 in the described case, meaning the new logic in AddTicketToDB will try to remove the existing ticket and add the new one. This is easy to fix and will make the logic just a little messier in one place or another, but I'm not sure there's any cleaner way to implement this without changing a bunch of other stuff.

[ZeroSkill1]

Yeah... I kinda missed that.

So we need the following logic:

When a ticket already exists with the same Title ID,

• Only replace if replace is true and the existing entry size matches the size of the entry that is being installed.
• Return failure if an entry was found and replace was not true or the entry sizes mismatch.

To that, I'd say this should do it:

if (!replace) {
    return 1;
}
else if (file_entry.size != size) {
    return 2;
}
else {
    do_replace = true;
    break;
}

It should immediately fail when replace is false so this should no longer apply:

Consider the case that replace is passed as false to AddTicketToDB, and there is an existing ticket in the filesystem with the passed title ID.

As the first check is seeing whether the caller wants replacing or not.

[aspargas2]

Looks good, that should work fine. Pretty sure this would be good to merge with that code changed to what you just proposed (and the elses in that code block moved on the same line as the }s to match the formatting of the rest of the repo, lol). Though I'd like @d0k3 or @Wolfvak to also look over this before merging to make sure nothing else is being missed, and that there's nothing that uses these functions and assumes the current behavior.

[eku]

For better readability, I would better define constants instead of numeric literals.

[ZeroSkill1]

Should all be good now.

[eku]

Should all be good now.

Any reason you did not define constants for 0 and 1, but for 2?

[aspargas2]

Because 0 and 1 are used to for success and failure returns all throughout the repo without defines. IMO, it's a little weird that we're using a special return value at all here, but it would be more weird to define 0 and 1 as generic success and failure to only use those defines in this one function.

[eku]

Because 0 and 1 are used to for success and failure returns all throughout the repo without defines. IMO, it's a little weird that we're using a special return value at all here, but it would be more weird to define 0 and 1 as generic success and failure to only use those defines in this one function.

Yes, I understood that. But for the one function that returns three different values, I would define the three values as constants, precisely because this function acts differently from the rest. For this one case, it also increases the readability of the code.

[d0k3]

Also merged, thanks for your continued contributions!