Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MCSTR() breaks providers lookups #70

Closed
CodaFi opened this issue Jun 1, 2013 · 10 comments
Closed

MCSTR() breaks providers lookups #70

CodaFi opened this issue Jun 1, 2013 · 10 comments
Labels

Comments

@CodaFi
Copy link
Member

CodaFi commented Jun 1, 2013

While we're at it, we should also synthesize the identifier property

@dinhvh
Copy link
Member

dinhvh commented Jun 1, 2013

It's not clear what this issue is about. Could you clarify?

@CodaFi
Copy link
Member Author

CodaFi commented Jun 1, 2013

Set a breakpoint in fillWithInfo() and you can see that all of the objectForKey() calls are failing because MCSTR() does "" "str" "" instead of "str"

@dinhvh
Copy link
Member

dinhvh commented Jun 1, 2013

When you're talking about code, it's more clear if you reference which file to look into. fillWithInfo() name is used at several locations.

Regarding the issue itself:

I don't think that's the reason of your issue. Keep investigating about your specific issue.
If MCSTR() was really broken, it would fail everywhere.

MCSTR() does "" str "", which concatenate an empty string with str with an empty string.
It makes sure that str is a constant.

@dinhvh
Copy link
Member

dinhvh commented Jun 1, 2013

Reopen if that's really the issue and give more details.

@dinhvh dinhvh closed this as completed Jun 1, 2013
@CodaFi
Copy link
Member Author

CodaFi commented Jun 1, 2013

But it fails specifically for the providers dictionary because the JSON keys aren't made with MCSTR(), they're just parsed out into it. It works everywhere else because everywhere else uses MCSTR().

Here, these lines fail for every provider HashMap given to them:

https://github.com/MailCore/mailcore2/blob/master/src/core/provider/MCMailProvider.cc#L62
https://github.com/MailCore/mailcore2/blob/master/src/core/provider/MCMailProvider.cc#L66

And because they fail, every one of the folder lookup methods afterwards dereferences a NULL pointer and causes a fatal crash.

@CodaFi CodaFi reopened this Jun 1, 2013
@CodaFi
Copy link
Member Author

CodaFi commented Jun 1, 2013

If I replace those MCSTR()'s with mailcore::String::stringWithUTF8Format() it works like a charm.

@dinhvh
Copy link
Member

dinhvh commented Jun 1, 2013

Therefore, that's an issue with String::uniquedStringWithUTF8Characters().
Do you have a minimal code that reproduces the issue?

Could you try to apply that patch?

diff --git a/src/core/basetypes/MCString.cc b/src/core/basetypes/MCString.cc
index 6710e59..2398958 100644
--- a/src/core/basetypes/MCString.cc
+++ b/src/core/basetypes/MCString.cc
@@ -1975,6 +1975,7 @@ String * String::substringWithRange(Range range)
}

static chash * uniquedStringHash = NULL;
+static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;

static void initUniquedStringHash()
{
@@ -1991,14 +1992,17 @@ String * String::uniquedStringWithUTF8Characters(const char * UTF8Characters)
pthread_once(&once, initUniquedStringHash);
key.data = (void *) UTF8Characters;
key.len = (unsigned int) strlen(UTF8Characters);

  • pthread_mutex_lock(&lock);
    r = chash_get(uniquedStringHash, &key, &value);
    if (r == 0) {
  •    pthread_mutex_unlock(&lock);
     return (String *) value.data;
    
    }
    else {
    value.data = new String(UTF8Characters);
    value.len = 0;
    chash_set(uniquedStringHash, &key, &value, NULL);
  •    pthread_mutex_unlock(&lock);
     return (String *) value.data;
    
    }
    }

@dinhvh
Copy link
Member

dinhvh commented Jun 2, 2013

Patch not broken by formatting is here:
https://gist.github.com/dinhviethoa/5692527
Let me know if it solves your issue.

@CodaFi
Copy link
Member Author

CodaFi commented Jun 3, 2013

Yep, that patch solves it. Sorry I couldn't answer you earlier, I was integrating the framework and had to fix a ton of warnings before I could test anything.

@dinhvh
Copy link
Member

dinhvh commented Jun 3, 2013

Fixed with b88d4be

@dinhvh dinhvh closed this as completed Jun 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants