All the filesystem-related W* macros accept a filesystem context pointer as first parameter

[falemagn]

For better portability.

[wolfSSL-Bot]

Can one of the admins verify this patch?

[JacobBarthelmeh]

ok to test

[JacobBarthelmeh]

Have been reviewing this, and looked it over a couple times so far. My only concern is backwards compatibility if in some case the applications had overridden the W* file types with their own, then they would need to update their application when updating the wolfSSH version used.

[falemagn]

Have been reviewing this, and looked it over a couple times so far. My only concern is backwards compatibility if in some case the applications had overridden the W* file types with their own, then they would need to update their application when updating the wolfSSH version used.

Our own application needs a filesystem context in all cases, how do you suggest to move forward? This topic is also covered by issue #558.

[ejohnstown]

Is there a chance you can pass the ssh->fs ctx into all the filesystem functions, and add an accessor to set the fs ctx?

[falemagn]

@ejohnstown I am not sure I understand. Which filesystem functions are we talking about? The accessor, as far as I can see and understand, is already there: wolfSSH_SetFilesystemHandle.

[ejohnstown]

@ejohnstown I am not sure I understand. Which filesystem functions are we talking about?

I meant the function-style macros you added. For the fs parameter you passed in NULL, rather than the intended ssh->fs.

The accessor, as far as I can see and understand, is already there: wolfSSH_SetFilesystemHandle.

Whoops. I see the function wolfSSH_SetFilesystemHandle(). That's my mistake.

[falemagn]

@ejohnstown I am not sure I understand. Which filesystem functions are we talking about?

I meant the function-style macros you added. For the fs parameter you passed in NULL, rather than the intended ssh->fs.

I passed NULL where NULL was already being passed to the other macros that already accepted the same parameter: those places make no use of the filesystem context, and I didn't change that.

As explained in issue #558, I believe the whole problem should be tackled differently, avoiding macros, but that issue hasn't got attention yet.

This patch simply goes in the same direction that has already been set.

[ejohnstown]

This should be rebased with the current master branch. And it breaks the wolfSSHd build. The changes aren't that bad.

[ejohnstown]

I am going to merge this despite the tests failed. I'm going to PR a fix right after merging this.