1

EDIT: Dear Future Readers, the std::string had nothing to do with the problem. It was an unterminated array.

In a nutshell, the problem is that adding a declaration of a single std::string to a program that otherwise contains only C causes the error "Access violation reading location 0xfffffffffffffffe."

In the code below, if the line where the std::string is declared is commented out, the program runs to completion without error. If the line however is left in the program (uncommented), the program crashes with the above stated Acess Violation error. When I open the running program in the VS2010 debugger, the Access Violation has occurred at the call to ldap_search_sA().

Notice that the declared std::string is never used. It doesn't have to be used for it to cause the access violation. Simply declaring it will cause the Access Violation.

My suspicion is it has nothing to do with the LDAP code, but I could be wrong.

int main() 
{
    try {
        // Uncommenting the next line causes an Access Violation 
        // at the call to ldap_search_sA().
        // std::string s;
        LDAP* pLdapConnection = ldap_initA("eu.scor.local", LDAP_PORT);
        ULONG version = LDAP_VERSION3;
        ldap_set_option(pLdapConnection, LDAP_OPT_PROTOCOL_VERSION, (void*) &version);         
        ldap_connect(pLdapConnection, NULL);
        ldap_bind_sA(pLdapConnection, NULL, NULL, LDAP_AUTH_NTLM);
        LDAPMessage* pSearchResult;
        PCHAR pMyAttributes[2];
        pMyAttributes[0] = "cn";
        pMyAttributes[1] = "description";
        ldap_search_sA(pLdapConnection, "dc=eu,dc=scor,dc=local", LDAP_SCOPE_SUBTREE,  "objectClass=computer)", pMyAttributes, 0, &pSearchResult);    
    } catch (...) {
        printf("exception\n");
    }
    return 0;
}
sbi
  • 219,715
  • 46
  • 258
  • 445
John Fitzpatrick
  • 4,207
  • 7
  • 48
  • 71
  • 2
    I don't know this LDAP stuff, but my suspicion is you invoke _Undefined Behavior_ somewhere and the definition[(!)](http://stackoverflow.com/questions/1410563/) of the string just makes it manifest as an AV. – sbi Dec 19 '11 at 10:12
  • OK, so you believe it is in fact improper use of the LDAP stuff and I should look deeper there? (No need to answer if it's "yes".) – John Fitzpatrick Dec 19 '11 at 10:16
  • Yes, I do believe that, and Banthar has meanwhile found out what is wrong. – sbi Dec 20 '11 at 16:15

3 Answers3

4
    PCHAR pMyAttributes[2];
    pMyAttributes[0] = "cn";
    pMyAttributes[1] = "description";

Attribute array should be NULL-terminated:

    PCHAR pMyAttributes[3];
    pMyAttributes[0] = "cn";
    pMyAttributes[1] = "description";
    pMyAttributes[2] = NULL;
Piotr Praszmo
  • 17,928
  • 1
  • 57
  • 65
2

I don't know what ldap_search_sA is, but the ldap_search function in OpenLDAP takes a pointer to a null pointer terminated array of char*. The array you are passing isn't correctly terminated, so anything may happen. I'd recommend using std::vector<char*> for this, in general, and wrapping the calls in a C++ function which systematically postfixes the terminator, so you don't forget. Although in such simple cases:

char* attributes[] = { "cn", "description", NULL };

will do the trick. It will probably provoke a warning; it really should be:

char const* attributes[] = { ... };

But the OpenLDAP interface is legacy C, which ignores const, so you'd need a const_cast at the call site. (Another argument for wrapping the function.)

Finally, I'd strongly advise that you drop the obfuscating typedefs like PCHAR; they just make the code less clear.

James Kanze
  • 150,581
  • 18
  • 184
  • 329
  • FWIW: I generally declare this sort of thing `static` and `const` (but the `const` will require a `const_cast` later). That is `static char const* const attributes[] = {...};`. I might add, with respect to your code: the one advantage with C style arrays is that you don't have to specify the size of the array. The compiler calculates it itself from the initialization values. – James Kanze Dec 19 '11 at 10:48
1

According to my experience, when weird things like this are observed in C++, what is in fact happening is that some piece of code somewhere corrupts memory, and this corruption may manifest itself in various odd ways, including the possibility that it may not manifest itself at all. These manifestations vary depending on where things are located in memory, so the introduction of a new variable probably causes things to be moved in memory just enough so as to cause a manifestation of the corruption where otherwise it would not be manifested. So, if I were in your shoes I would entirely forget about the string itself and I would concentrate on the rest of the code, trying to figure out exactly what you do in there which corrupts memory.

I notice that you invoke several functions without checking their return values, even though it is not in the spec of these functions to throw exceptions. So, if any of these functions fails, (starting with ldap_initA,) and you proceed assuming that it did not fail, you may get memory corruption. Have you checked this?

Mike Nakis
  • 56,297
  • 11
  • 110
  • 142
  • Thanks Mike you are correct. My unterminated array was the culprit. I did originally check all return values but took them out to make the posted code more compact. Funny though is that without the std::string DEFINITION (not declaration) the unterminated array is no problem. You really have to be good at C/C++ to use it. Not for lightweights like me... – John Fitzpatrick Dec 19 '11 at 10:27
  • Yes, and of course @Banthar deserves the credit for knowing precisely what the problem was by just looking at the code. (I did not know because I am not familiar with LDAP.) – Mike Nakis Dec 19 '11 at 10:41