I'm confused by this -- strcmp shouldn't return 0 when comparing "Sans" to "Sans Bold". Are you sure you have the analysis of your problem right?

You can try it out yourself and see it doesn't work properly.
E.g. download Source Sans Pro, extract 'SourceSansPro-Semibold.otf' and try to use appendSFNTName to change the name from 'Source Sans Pro Semibold' to 'Source Sans Pro'. And you'll see it doesn't work:

$ python Code/font-name-fix.py -f 'Source Sans Pro' .local/share/fonts/adobe-source-sans/SourceSansPro-Semibold.otf
The following table(s) in the font have been ignored by FontForge
  Ignoring 'DSIG' digital signature table
$ fc-list | grep -i 'Source Sans Pro'
/home/user/.local/share/fonts/adobe-source-sans/SourceSansPro-Semibold.otf: Source Sans Pro,Source Sans Pro Semibold:style=Semibold,Regular

The font name stays 'Source Sans Pro,Source Sans Pro Semibold'.

But after applying this patch it works properly:

$ python Code/font-name-fix.py -f 'Source Sans Pro' .local/share/fonts/adobe-source-sans/SourceSansPro-Semibold.otf
The following table(s) in the font have been ignored by FontForge
  Ignoring 'DSIG' digital signature table
$ fc-list | grep -i 'Source Sans Pro'
/home/user/.local/share/fonts/adobe-source-sans/SourceSansPro-Semibold.otf: Source Sans Pro:style=Semibold,Regular

Now the font name is just 'Source Sans Pro', as intended.

Here's the python script I used: font-name-fix.txt (just change extension to .py)

It looks like the solution is slightly different. SetSFNTName() is not called with the real English name values, but rather with default values produced earlier by DefaultTTFEnglishNames(). So when the current value differs from default, but the target value is the default, strcmp() does receive two identical strings and produces 0.

So for the English names the logic should be as follows:

1. The new name is empty ==> delete entry
2. The new name equals to default ==> delete entry
3. The new name differs from default ==> add or overwrite entry

Instead of deleting the if clause entirely we should do

	if ( names!=NULL ) {
	    free( names->names[strid] );
	    names->names[strid] = NULL;
	}
	Py_DECREF(val);

It looks like the solution is slightly different. SetSFNTName() is not called with the real English name values, but rather with default values produced earlier by DefaultTTFEnglishNames(). So when the current value differs from default, but the target value is the default, strcmp() does receive two identical strings and produces 0.

So for the English names the logic should be as follows:

1. The new name is empty ==> delete entry

2. **The new name equals to default ==> delete entry**

3. The new name differs from default ==> add or overwrite entry

Instead of deleting the if clause entirely we should do

	if ( names!=NULL ) {
	    free( names->names[strid] );
	    names->names[strid] = NULL;
	}
	Py_DECREF(val);

Yeah, that does also seem to work for me.

Removing the if-statement entirely or doing it your way both seem to work for fixing #2635, so either approach is fine with me. If your way is implemented, then this pull request can be closed.

@iorsh Looks like the author would rather we implement the suggested fix rather than changing this PR. What do you think?

@iorsh Looks like the author would rather we implement the suggested fix rather than changing this PR. What do you think?

I'm ok with that. Superseding this PR with #5494