329017 - Automatically produce application code for error code to string mapping

Status: RESOLVED INACTIVE

(NSS :: Tools, defect, P5)

Description

[Kai Engert (:KaiE:)]

We have some requests to make PSM produce better error messages.

I would like to propose that we define a master location for error messages.

In directory mozilla/security/nss/cmd/lib I found 3 files, NSPRerrs.h, SECerrs.h and SSLerrs.h that seem to contain complete lists of all NSPR and NSS error codes and messages.

I would like to propose this directory becomes the master location.

This is assuming that we are allowed to improve those strings and make them appropriate for both server and client applications.

Assuming you agree with my proposal, I suggest we use an automatic way to include these error messages in the PSM application.

I will add a patch to this bug.
It adds a new command line utility to NSS, named "psmmsgs", short for "PSM messages".

Driven by a command line parameter, this utility produces C/C++ code fragments that can be copied and pasted into application code.

(For now I have started to produce and copy the output manually. In the future, an application might chose to automate production of code fragments and inclusion in application sources.)

For now I started with two styles of output.

-b will produce lines that are appropriate for Mozilla's "string bundles". Each line contains an identifier and the human readable english message. (FYI: These are not C/C++ files, but plain text files that can be translated in localized versions)

-c will produce C "case" output, that maps C error identifiers to the string identifiers (as produced by -b)

This command line utility can easily be enhanced to produce other output suitable for different applications.

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Patch v1

Nelson, what do you think about my proposal and this patch?

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Kai, I see that you have already found bug 172051.
You asked a question there, and I will answer it there.  
That answer may affect the feasibility of the approach in this bug.

If the scheme of bug 172051 was already implemented in NSS/FF/TB/etc, 
would you prefer to use it than the scheme in this bug?  or not?

Would the scheme of bug 172051 obviate the huge file of strings 
that your proposed new psmmsgs program would create?  
e.g. the file shown in https://bugzilla.mozilla.org/attachment.cgi?id=213633 

Comment 3

[Kai Engert (:KaiE:)]

(In reply to comment #2)
> If the scheme of bug 172051 was already implemented in NSS/FF/TB/etc, 
> would you prefer to use it than the scheme in this bug?  or not?
> 
> Would the scheme of bug 172051 obviate the huge file of strings 
> that your proposed new psmmsgs program would create?  
> e.g. the file shown in https://bugzilla.mozilla.org/attachment.cgi?id=213633 


In consider he question, whether the application uses the API described in bug 172051 to deliver error messages, independent of where the strings live.

I only read bug 172051 after I had worked on this one.

It might be a good idea to make localized error messages available using the API described in bug 172051, but this raises the new problem of solving a mapping issue between NSPR's "language number" and the locale identifiers in use by the remainder of the Mozilla application (I don't know how that works, but I'm pretty sure it's different).

So even if we agree to solve that issue and leverage the C API, this only avoids the large switch/case block in that patch, but it does not eliminate the need to duplicate the error messages into some localizable text file.

There are two reasons why we have that requirement.

First, the string tables must be accessible for 30+ teams of localizers. Those are people who don't want to deal with C code, but simple text files. They might even use helper tools to work with those text files, so our string tables better use the standard approach, if we want them localized.

Second, the Mozilla applications support loadable language packs. They can be installed by downloading a package from the web, and this package is not binary, not compiled. Our error messages should be included in such a package, when a user installs a pack.

So in my opinion, we definitly need to provide all error messages in the application prefered way and use that approach dynamically at runtime. We could provide these strings using the API mentioned in bug 172051, but I'm not sure whether that gives us any advantage, since I don't see anybody currently using that API in mozilla sources.

I'm fine to make the strings available through that API in addition, but that will rise additional requirements to solve. For example, we must transform the dynamic string table into a static buffer as required by the C API, at runtime. That will take a bit of time, but we should make sure not to influence the application startup performance in a negative way. So we'd need additional logic to possibly delay producing such a table until the first error message is required.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 213632 [details] [diff] [review]
Patch v1

I really cannot give r+ to this patch, because doing so would forever
lock NSS into the requirement of preserving these 3 header files which
presently are all in nss/cmd/lib:

>+#include "../lib/SSLerrs.h"
>+#include "../lib/SECerrs.h"
>+#include "../lib/NSPRerrs.h"

I firmly intend to get rid of NSPRerrs.h and begin using NSPR's own
built-in table of error strings, as documented in bug 172051.

I'm willing to keep the other two header files, even when I do fix
bug 172051.  But they will be moved out of nss/cmd/lib and into 
somewhere else, such as nss/lib/util.  (IIRC, bug 172051 has a 
patch that shows this.)  

So, I consider this bug blocked by bug 172051.  
I hope to work on that bug very shortly after finishing the ECC work
I am now doing.  But I cannot guarantee that bug 172051 will be 
resolved in time for FF 2.0 (or whatever upcoming product release 
wants this bug to be resolved.

Comment 5

[Kai Engert (:KaiE:)]

Thanks Nelson, that's ok with me.

Comment 6

[Kai Engert (:KaiE:)]

Attachment: Patch v2

My original intention was to produce the error messages for NSS errors.

The only reason for me to add NSPR error codes was completeness.
But as NSPR is causing a problem for this approach, I'm more than willing to skip that.

I will attach a new patch that has the NSPR include removed.

I made one more change, as it has been proposed in bug 107491 comment 36: I'm removing the numeric error code suffix from the string identifiers.

Nelson, you don't need to promise that these header files will always stay around. We can always change this tool to obtain the strings from a new location.

I'm not proposing an automated mechanism - I'm proposing a semi-automatic approach. We should manually re-run this tool and merge new things into PSM when we believe it's necessary.

We can either check this tool in, or just keep this tool attached to this bug only - as long as we agree to use an approach like this for complete error message inclusion.

(and we seem to agree if I read your comments in bug 107491)

Comment 7

[Kai Engert (:KaiE:)]

Attachment: Patch v3

Small fix compared to v3:

Changed a C++ style comment to a C style comment.

Comment 8

[Kai Engert (:KaiE:)]

This bug waits for Nelson's review.

Nelson said 3 years ago in comment 4 that this bug is blocked by bug 172051, but IMHO there is no real reason to. Once bug 172051 gets changed, the relevant code added here can then be updated. It sounds like that will be a very small adjustment.

Comment 10

[Kai Engert (:KaiE:)]

Attachment: Patch v4

merged to trunk