Resolve full path of pcap which starts with '~'

[rajeshsingh381]

expand paths like '~/path/to/file.pcap' to '/home/myuser/path/to/file.pcap'

[rajeshsingh381]

Really wish this feature gets added in next release. To that end, any suggestions/comments to code implementation are welcome, I will try my best to cater them,

[rajeshsingh381]

Hi @wdoekes , could you please take a look at this PR when you find some time and offer your suggestions, if any? I am new to github community, so please also do let me know if a direct ping is inappropriate, I will refrain from in future.

[wdoekes]

Maybe add expanduser(). That way ~someuser/blah would also work, which will be expected.

Chatgpt was helpful enough to give me this ("can you show me a C implementation of python os.path.expanduser?"):

#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <pwd.h>

char* expanduser(char* path) {
    if (path == NULL) {
        return NULL;
    }
    
    if (path[0] == '~') {
        char* homedir = NULL;
        
        if (path[1] == '\0' || path[1] == '/') {
            homedir = getenv("HOME");
        } else {
            struct passwd* pw = getpwnam(path+1);
            
            if (pw != NULL) {
                homedir = pw->pw_dir;
            }
        }
        
        if (homedir != NULL) {
            char* expanded = malloc(strlen(homedir) + strlen(path));
            sprintf(expanded, "%s%s", homedir, path+1);
            return expanded;
        }
    }
    
    return strdup(path);
}

with these caveats:

Note that this implementation uses malloc() and free() to allocate and deallocate memory for the expanded path, so you'll need to remember to free the returned string when you're done with it.

(I'd probably prefer a version with a provided buffer.)

(Also, it uses a non-reentrant getpwnam. Look into getpwnam_r instead..)

[wdoekes]

            sprintf(expanded, "%s%s", homedir, path+1);

Hehe. This can't possibly work for ~someuser unless the username length is added too; otherwise we get /home/someusersomeuser/bla

(Also.. no malloc() NULL check. But I don't want any malloc here anyway.)

[rajeshsingh381]

Thankyou @wdoekes for your suggestions, I have added support for ~/someuser/blah, ~someuser/blah and ~someuser cases and tested them. Please let me know if you have any suggestions/inputs.

[wdoekes]

Please check my suggestion for the expand_user_path invocation. It should not be optional based on ~, but always be called directly.

[wdoekes]

FYI: I moved find_file() to fileutil.c where is used by both pcap and rtpstream file lookups.

[rajeshsingh381]

Sorry for inactivity, I have applied the changes you suggested, and will verify the them over the different cases you mentioned tomorrow

[rajeshsingh381]

Hi, have verified the new changes, for the mentioned 3 cases. I tried to reproduce and debug the regression test failures of github-#259 and github-#196, but weirdly they don't happen when I run the SIPp commands manually, call is successful with 0 return code.

[rajeshsingh381]

Hi, apologies for inactivity. I have fixed the regression test failures.

[lemenkov]

LGTM and it passes the tests.

[wdoekes]

Thanks for the style fixes 👍

I'm unfortunately unable to review this timely. Unassigning. Leaving the review+merge to someone else. E.g. @lemenkov

[orgads]

Dynamic array size is a GNU extension. Do we have this elsewhere in the codebase?

[rajeshsingh381]

Thanks, I was not aware of this and couldn't find such examples in the codebase either. Now using malloc() reluctantly.

[rajeshsingh381]

Hi @orgads could you please review the updated diff when you get some time. Thankyou.

[orgads]

Thanks for the reminder.

I approve, just reply on the NULL comment and then I'll merge.

[orgads]

You have conflicts. Please rebase.