Change default username to empty string

[tox-user]

Fixes #768.

---

This change is 

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[zoff99]

memcpy here is a bit odd.
just use
name[0]='\0';

or memset it to all zeros for MAX_NAME_LENGTH
or just leave name alone since we are returning "0" as len

[zoff99]

and return 0; is missing for early return

[zoff99]

see:
#768 (comment)

[nurupo]

Um, @tox-user, do you realize that memory copy of 0 length doesn't copy anything? i.e. you might as well just remove that function call as it does nothing.

Also, Tox defines "empty" data, i.e. empty name, as arrays of 0 length, so just setting length to 0 should be enough, it shouldn't matter what the name array contains if the length is 0, it might as well just say "install gentoo" for all intents and purposes, because clients are supposed to read exactly 0 bytes of that name (i.e. none). Also, toxcore doesn't null-terminate data/strings in the API, but I guess you could.

[sudden6]

I'm with @nurupo here, neither memcpy nor name[0]=0 are correct here.

[zoff99]

memcpy is wrong.
why not be nice and put a null byte there. it is utf-8 ?

    if (peer->nick_len == 0) {
        name[0] = '\0';
        return 0;
    }

this seems the only correct version to me.
why be lazy and leave the buffer as is?

[iphydf]

Just return 0 is enough. If you get 0, you can read exactly 0 bytes from the buffer, so it doesn't matter what's in the first position that you never read.

[zoff99]

still we could be nice about it

[sudden6]

@zoff99 that's not being nice, it's writing a byte into a buffer that contains no bytes. These values aren't C Strings that need to be \0 terminated.

[SkyzohKey]

The real question is « Why this piece of code exists? » Can't we just make sure the real peer name is ALWAYS in sync?

EDIT: @tox-user Please sign your commits using GPG. ;)

[nurupo]

@tox-user btw, this doesn't fix #768. No name (empty name, name of length 0) is a valid name, so clients wouldn't be able to distinguish between users whose names they don't yet know and who have empty name set as their name. Tox just doesn't seem to have a way to indicate "name not known".

[Diadlo]

@nurupo Is it possible to set minimal name length == 1?

[zoff99]

@Diadlo will setting min len to 1 not break some clients, or at least compatibility?

another question would be does a client need to distinguish between "name not yet known" and "empty string name" ?
the current "Tox User" things seems just wrong to me.

[sudden6]

I think the cases "no name sent yet" or "no name set" might as well be treated the same.

[SkyzohKey]

Or clients could stop acting in a stupid way and set the name at profile creation. As Ricin did, as Konv does.

This MUST be enforced at toxcore level IMHO.

Some sort of tox_create_profile(path, username[, password, passwordConfirm]) and then a tox_load_profile(path, username[, password]) aswell as tox_save_profile(path). You could add tox_load_tmp_profile() aswell for not disk-saved profiles too.

Just an idea but it would prevent corrupted profiles when a client does not handle everything right (as Ricin did in the start of it's life xD).

Would be nice if we could all agree that Tox design about profiles handling/managment is fundamentally wrong and instead of trying to patch it we could make it better.

[tox-user]

@zoff99
Done.

@nurupo
I agree that isn't not a perect solution, but I think it's better than nothing. What else could be done? Maybe an empty name shouldn't be allowed?

@SkyzohKey
It should always be in sync. I don't know how to achieve that though. I did sign my commit with GPG but github doesn't like my email address and says it's "unverified". Any idea what I can do about it? I don't use my real email address in git.

[SkyzohKey]

@tox-user Just create a new email address that you use for github & set it as your principal email address so that you can sign and it'll says Verified. ;)

[zoff99]

---

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

toxcore/group.c, line 882 at r1 (raw file):

Previously, sudden6 wrote…

@zoff99 that's not being nice, it's writing a byte into a buffer that contains no bytes. These values aren't C Strings that need to be \0 terminated.

hh? the buffer must contain at least MAX_NAME_LENGTH bytes. the API says so.
and since it must be utf-8 what else than bytes should there be inside the buffer?

---

Comments from Reviewable

[tox-user]

@SkyzohKey I have a real email address in my github account and it's verified. I think github doesn't want me to use a fake email address in git. It wants me to use an email that is added to my github account, but it doesn't let me add my fake address and I don't know how I would verify it. Why do I even have to? I can't be the only one who uses a fake email in git and signs commits with a key that that uses the same fake email? It must be possible.

[sudden6]

@tox-user have you checked that your key is actually available on some major keyservers? ie have you tried to download it from there?

[iphydf]

Please enable the checkbox "Allow edits from maintainers." on the bottom right. Then I can rebase your PR and sign the commit myself.

[tox-user]

@sudden6
I have. I needed to change my email used in the GPG key and in my commit for github to accept it.

@iphydf
It is enabled, but I fixed the GPG now.

[nurupo]

I agree that isn't not a perect solution, but I think it's better than nothing. What else could be done? Maybe an empty name shouldn't be allowed?

Maybe it shouldn't be allowed, or maybe there should be a special value for "no name received yet" like a error code enum in the API, or maybe toxcore should avoid telling a client that there is a user until it gets all information about the said user. I'm more in the favor of a special value / error code.