Re: [libvirt] [PATCH v2 2/2] libssh_transport: add new libssh-based transport

> > > + memset(&retr_passphrase, 0, sizeof(virConnectCredential)); > > > + retr_passphrase.type = virCredTypeForPrompt(sess->cred, echo); > > > + retr_passphrase.prompt = virBufferCurrentContent(&buff); > > > > This shouldn't really be used. Please get the content into a variable. > > Not a problem to change this, but could you please explain a bit more > on the reasons? (So I can avoid doing the same again.) Basically any call any function that uses the buffer invalidates the pointer returned by virBufferCurrentContent (or should be treated as such) which is prone to bugs when later modifying the code. Most of the uses of the function are in special cases (or should be removed).

> > > +int > > > +virNetLibsshSessionConnect(virNetLibsshSessionPtr sess, > > > + int sock) > > > +{ > > > + int ret; > > > + const char *errmsg; > > > + > > > + VIR_DEBUG("sess=%p, sock=%d", sess, sock); > > > + > > > + if (!sess || sess->state != VIR_NET_LIBSSH_STATE_NEW) { > > > + virReportError(VIR_ERR_LIBSSH, "%s", > > > + _("Invalid virNetLibsshSessionPtr")); > > > + return -1; > > > + } > > > + > > > + virObjectLock(sess); > > > + > > > + /* check if configuration is valid */ > > > + if ((ret = virNetLibsshValidateConfig(sess)) < 0) > > > + goto error; > > > + > > > + /* read ~/.ssh/config */ > > > + if ((ret = ssh_options_parse_config(sess->session, NULL)) < 0) > > > + goto error; > > > + > > > + /* set the socket FD for the libssh session */ > > > + if ((ret = ssh_options_set(sess->session, SSH_OPTIONS_FD, &sock)) < 0) > > > > Is this guaranteed to copy the socket number at call time? Otherwise > > (similarly to the ones above will not work reliably). > > I don't understand this sentence (it seems truncated), can you please > rephrase it? Sorry I probably got sidetracked and did not finish my thought. My question was whether ssh_options_set accesses the pointer to 'sock' right away and copies it. You are passing a pointer to a stack allocated variable which will get out of scope later, thus the pointer should not be accessed after the call to ssh_options_set returns.