[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
On Fri, May 01, 2009 at 01:57:31PM -0700, Jordan Brown wrote: > Nicolas Williams wrote: > >But I thought it was clear that using a CPP macro to rename that symbol > >was the way to go. > > I thought so, but became less happy with that when I realized that % lines > were duplicated in all of the generated code, not just in the *_svc.c file. > Including a #define syslog in the generated *.h file would be bad. You That's what CPPFLAGS/CFLAGS in Makefiles are for.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
Nicolas Williams wrote: >> Actually, there's one variant that would rely only on arguably documented >> features: if you use -L and interpose on syslog. Interposing on libc >> functions just makes me queasy. > > But I thought it was clear that using a CPP macro to rename that symbol > was the way to go. I thought so, but became less happy with that when I realized that % lines were duplicated in all of the generated code, not just in the *_svc.c file. Including a #define syslog in the generated *.h file would be bad. You can work around this by putting the % lines inside #if defined(RPC_SVC), but that's starting to get kind of ugly. You might think that you could -Dsyslog=mylog when you compile *_svc.c, but then you don't have an extern declaration for mylog. >> All of the other variations involve macros and those make assumptions about >> the structure of the generated file. > > I don't agree. The XDR input to rpcgen is definitely stable, if not > even Committed Standard at least in part (since it is documented in a > standards-track RFC). Er... I said the *generated* file, the output, not the input. Agreed, the RPCL input is stable. What is far from stable is the generated C code, which is where the message references are. Note that one thing about the RPCL is documented as unstable - where % lines appear in the output: http://docs.sun.com/app/docs/doc/802-1997/6i6091lam?a=view rpcgen provides an additional preprocessing feature: any line that begins with a percent sign (%) is passed directly to the output file, with no action on the line's content. Caution is required because rpcgen does not always place the lines where you intend. Check the output source file and, if needed, edit it.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
On Fri, May 01, 2009 at 09:24:32AM -0700, Jordan Brown wrote: > Nicolas Williams wrote: > >On Fri, May 01, 2009 at 08:46:13AM -0700, Jordan Brown wrote: > >>Nicolas Williams wrote: > >>>In other words: it's safe to use any _documented_ feature of rpcgen. > >>Right, and these various possibilities all depend on _undocumented_ > >>features and behaviors. > > > >I'm not seeing what undocumented feature you'll be relying on. > > Actually, there's one variant that would rely only on arguably documented > features: if you use -L and interpose on syslog. Interposing on libc > functions just makes me queasy. But I thought it was clear that using a CPP macro to rename that symbol was the way to go. > All of the other variations involve macros and those make assumptions about > the structure of the generated file. I don't agree. The XDR input to rpcgen is definitely stable, if not even Committed Standard at least in part (since it is documented in a standards-track RFC). > >Even if you were, if it's a "behavior that rises to the level of an > >interface" then you're still safe. > > > >If in doubt then ask ARC members if it's worth running a case. > > Yes on both counts.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
On Fri, May 01, 2009 at 08:46:13AM -0700, Jordan Brown wrote: > Nicolas Williams wrote: > >In other words: it's safe to use any _documented_ feature of rpcgen. > > Right, and these various possibilities all depend on _undocumented_ > features and behaviors. I'm not seeing what undocumented feature you'll be relying on. Even if you were, if it's a "behavior that rises to the level of an interface" then you're still safe. If in doubt then ask ARC members if it's worth running a case.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
Nicolas Williams wrote: > On Fri, May 01, 2009 at 08:46:13AM -0700, Jordan Brown wrote: >> Nicolas Williams wrote: >>> In other words: it's safe to use any _documented_ feature of rpcgen. >> Right, and these various possibilities all depend on _undocumented_ >> features and behaviors. > > I'm not seeing what undocumented feature you'll be relying on. Actually, there's one variant that would rely only on arguably documented features: if you use -L and interpose on syslog. Interposing on libc functions just makes me queasy. All of the other variations involve macros and those make assumptions about the structure of the generated file. > Even if you were, if it's a "behavior that rises to the level of an > interface" then you're still safe. > > If in doubt then ask ARC members if it's worth running a case. Yes on both counts.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
Nicolas Williams wrote: > In other words: it's safe to use any _documented_ feature of rpcgen. Right, and these various possibilities all depend on _undocumented_ features and behaviors.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
On Thu, Apr 30, 2009 at 07:23:56PM -0700, Jordan Brown wrote: > Nicolas Williams wrote: > >On Thu, Apr 30, 2009 at 04:02:36PM -0700, Jordan Brown wrote: > >>I would accept that as an answer for my problem. That'd be nice - no > >>code changes, just documenting the idiom. Formally it's still take an > >>ARC case to guarantee that it'd work. > > > >Why? To guarantee that this behavior of rpcgen does not change? It's > >not already effectively Committed? PSARC's take would be that rpcgen is > >Committed. > > Every aspect of it? It's really pretty rare that every aspect of a command > is Committed. That's not how PSARC acts with respect to interfaces like this. First you'd have to do ARCeology to find if any ARC case exists for rpcgen, and if you do find one, then map the old interface stability to the new taxonomy, and either way, PSARC would look at the history of the thing. Finally, there's no need to run an ARC case for this. Instead, if anyone wanted to change rpcgen in a backwards incompatible way it would be their burden to show that such a change should be allowed and to update whatever consolidations to fix any uses of the broken interfaces. In other words: it's safe to use any _documented_ feature of rpcgen. Nico --
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
Nicolas Williams wrote: > On Thu, Apr 30, 2009 at 04:02:36PM -0700, Jordan Brown wrote: >> I would accept that as an answer for my problem. That'd be nice - no code >> changes, just documenting the idiom. Formally it's still take an ARC case >> to guarantee that it'd work. > > Why? To guarantee that this behavior of rpcgen does not change? It's > not already effectively Committed? PSARC's take would be that rpcgen is > Committed. Every aspect of it? It's really pretty rare that every aspect of a command is Committed. It'd be kind of stupid, but is it really a Committed interface that rpcgen's generated output never uses "syslog" as a local variable? (Actually, that'd probably be OK, since your redefinition would probably be just as good. Finding a fatal case is left as an exercise for the reader.) Where are % lines emitted in the output? Is it before the bulk of the generated code? I don't find it in the man page, but I'm pretty sure that I read somewhere that it's explicitly unspecified. If I thought that the exact form of the generated code was Committed, I could just postprocess it with sed to remove _msgout or replace the calls to it, but I don't think that would be wise. --- Foo. % lines are emitted for all types of output files. Putting "#define syslog mylog" into the generated *.h file would be a really bad idea. My "#define _msgout mylog" would be better, but still wouldn't be good. Back to the drawing board. Hmm. Maybe sed 's/syslog(/mylog(/'. Bleah.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
On Thu, Apr 30, 2009 at 04:02:36PM -0700, Jordan Brown wrote: > I would accept that as an answer for my problem. That'd be nice - no code > changes, just documenting the idiom. Formally it's still take an ARC case > to guarantee that it'd work. Why? To guarantee that this behavior of rpcgen does not change? It's not already effectively Committed? PSARC's take would be that rpcgen is Committed.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
On Thu, Apr 30, 2009 at 02:47:06PM -0700, Jordan Brown wrote: > Nicolas Williams wrote: > >There's one more option: let it use syslog(), but instead of interposing > >use a macro to force the use of the application's error handling > >function (with same prototype as syslog()). > > Remember that syslog is a varargs function. Unless you're willing to make > assumptions about how rpcgen uses it (which, granted, aren't all that > unreasonable; it always uses the same variation) or are willing to require > C99, you can't override it with a macro. You don't have to define a macro that takes arguments... You're only renaming the symbol.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
It's an interesting subject eh ? :-) My inclination would be to integrate the lint/cstyle changes, can't do any harm right ? .. WRT logging, Dtrace static probes keeps flashing in my mind for some odd reason.. Robert.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
On Thu, Apr 30, 2009 at 02:20:38PM -0700, Jordan Brown wrote: > Best is to arrange that the file can always be dynamically generated. As > it stands, the message handling mechanism makes that awkward. It looks > like you have three choices: accept rpcgen's undocumented error handling, > use -L and accept its syslog use, or use -L and interpose on syslog. None > of those give me warm fuzzy feelings; I'd like more direct control. There's one more option: let it use syslog(), but instead of interposing use a macro to force the use of the application's error handling function (with same prototype as syslog()). Nico --
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
Jordan Brown wrote:
> I'm told that you folks are one of the better places to ask about
> rpcgen. Please feel free to direct me elsewhere or forward as
> appropriate.
>
> I've inherited a program that uses RPC.
>
> For unknown historical reasons, its *_svc.c file has been modified and
> checked into the repository. This is a nuisance because when I change
> the *.x file and regenerate the *_svc.c file, I have to hand-merge the
> changes.
Why not remove the *_svc.c files from source control?
Are there changes other than the ones to make it pass cstyle and lint?
Remember that these are moving targets as well.
>
> The vast majority of the changes are to make the file pass cstyle and
> lint. It seems like it would be straightforward to fix rpcgen to
> generate cleaner code, so that the generated code would be as similar
> as possible to the modified-and-checked-in code, to minimize the work
> involved in a resync. That seems like a relatively non-controversial
> bit of work. (Any disagreement?)
>
> I did that work, and cleaned up my existing *_svc.c file to eliminate
> spurious differences between it and the generated file, and found that
> I only had one difference left, one difference that prevented me from
> just dynamically generating the file (which would avoid the need to
> fix the nits mentioned above, though I'd probably fix them anyway).
> The generated *_svc.c file includes a message handling function
> _msgout that calls syslog or fprintf(stderr, ...) depending on a
> couple of flags. My application has its own logging infrastructure,
> so I don't want to have this one file doing its own thing.
>
> I'd like to add a feature to rpcgen to allow the user to customize
> message handling, so that the generated file can be used directly and
> there is no need to hand-tweak it and check the file into the repository.
>
> The change that I have prototyped is to surround the definition of
> _msgout with
> #if!defined(_msgout)
> ...
> #endif
>
> This then allows one to supply an alternate definition using % lines
> in the *.x file, e.g.:
>
> %#define _msgout my_msgout
> %extern void my_msgout(char *);
>
> or perhaps
>
> %#define _msgout(s)printf("RPC: %s\n", (s))
>
> I could make this change as a consolidation-private interface, but it
> seems like it's of more general interest, suggesting an ARC case. At
> that point it seems like I should involve the owners... which is what
> brings us here.
>
> Any thoughts?
> ___
> nfs-discuss mailing list
> nfs-discuss at opensolaris.org
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
Nicolas Williams wrote: >> Remember that syslog is a varargs function. Unless you're willing to make >> assumptions about how rpcgen uses it (which, granted, aren't all that >> unreasonable; it always uses the same variation) or are willing to require >> C99, you can't override it with a macro. > > You don't have to define a macro that takes arguments... You're only > renaming the symbol. Ah, duh. Yes: you could do that with three public interfaces: 1) That rpcgen uses syslog for this purpose (which is already defined by -L) 2) That rpcgen does not use syslog for other purposes 3) That a %#define line is interposed at the right place to affect the syslog calls in the generated code, but not to affect references in the included header files. (That's why -Dsyslog=mylog isn't good enough, because it could well cause problems with syslog.h.) I would accept that as an answer for my problem. That'd be nice - no code changes, just documenting the idiom. Formally it's still take an ARC case to guarantee that it'd work.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
Nicolas Williams wrote: > There's one more option: let it use syslog(), but instead of interposing > use a macro to force the use of the application's error handling > function (with same prototype as syslog()). Remember that syslog is a varargs function. Unless you're willing to make assumptions about how rpcgen uses it (which, granted, aren't all that unreasonable; it always uses the same variation) or are willing to require C99, you can't override it with a macro. With C99, yes, that seems plausible. Much the same effect as interposing, except without the global effect. The idiom should be documented so that people know what they can and cannot rely on.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
Nicolas Williams wrote: > On Thu, Apr 30, 2009 at 10:53:50AM -0700, Jordan Brown wrote: >> The vast majority of the changes are to make the file pass cstyle and lint. > > That became necessary only once it became necessary to not generate this > file every time. It's NOT necessary to make code generators generate > cstyle-clean code. Yes... but we have observed in real life at least one case where people had to hand-tweak the generated code, which changes the equation considerably. > I don't think you should go touch rpcgen to make it generate > cstyle-clean code. Any reason other than development+test time and associated risk? > Ah! Then we should interpose on syslog() or punt. As you know from previous discussions, I don't like interposing on random libc functions. Yes, it can be done sort of safely, but it seems like an opportunity for problems and confusion. >> I'd like to add a feature to rpcgen to allow the user to customize message >> handling, so that the generated file can be used directly and there is no >> need to hand-tweak it and check the file into the repository. > > That's fine too, but if it's easier to punt or interpose on syslog() > then do that. It wasn't hard, and (as may be obvious from the fact that I raised the question) I liked it better.
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
Tom Haynes wrote:
> Jordan Brown wrote:
>> I'm told that you folks are one of the better places to ask about
>> rpcgen. Please feel free to direct me elsewhere or forward as
>> appropriate.
>>
>> I've inherited a program that uses RPC.
>>
>> For unknown historical reasons, its *_svc.c file has been modified and
>> checked into the repository. This is a nuisance because when I change
>> the *.x file and regenerate the *_svc.c file, I have to hand-merge the
>> changes.
>
> Why not remove the *_svc.c files from source control?
> Are there changes other than the ones to make it pass cstyle and lint?
It's got other differences - in particular, it doesn't include the _msgout
function. It's been hand edited to call the application's logging function
instead.
> Remember that these are moving targets as well.
True. My immediate reaction is that these files are not affected by that
any more or less than any other source files, but that's not true - fixing
a random source file to match a new cstyle requirement is just a matter of
fixing that file, where here it's necessary to both fix the file and fix
rpcgen, and get the fixed rpcgen onto build machines.
Best is to arrange that the file can always be dynamically generated. As
it stands, the message handling mechanism makes that awkward. It looks
like you have three choices: accept rpcgen's undocumented error handling,
use -L and accept its syslog use, or use -L and interpose on syslog. None
of those give me warm fuzzy feelings; I'd like more direct control.
If I can find a way out of the message handling dilemma I'm happy to let
somebody else who can't use dynamic generation worry about cstyle and lint.
Code hygiene does suggest that lint cleanliness is a good thing, so that
people can do proper lint checks against their entire application,
including generated code. That's the minority of the changes here, though;
most were for cstyle cleanliness.
>> The vast majority of the changes are to make the file pass cstyle and
>> lint. It seems like it would be straightforward to fix rpcgen to
>> generate cleaner code, so that the generated code would be as similar
>> as possible to the modified-and-checked-in code, to minimize the work
>> involved in a resync. That seems like a relatively non-controversial
>> bit of work. (Any disagreement?)
>>
>> I did that work, and cleaned up my existing *_svc.c file to eliminate
>> spurious differences between it and the generated file, and found that
>> I only had one difference left, one difference that prevented me from
>> just dynamically generating the file (which would avoid the need to
>> fix the nits mentioned above, though I'd probably fix them anyway).
>> The generated *_svc.c file includes a message handling function
>> _msgout that calls syslog or fprintf(stderr, ...) depending on a
>> couple of flags. My application has its own logging infrastructure,
>> so I don't want to have this one file doing its own thing.
>>
>> I'd like to add a feature to rpcgen to allow the user to customize
>> message handling, so that the generated file can be used directly and
>> there is no need to hand-tweak it and check the file into the repository.
>>
>> The change that I have prototyped is to surround the definition of
>> _msgout with
>> #if!defined(_msgout)
>> ...
>> #endif
>>
>> This then allows one to supply an alternate definition using % lines
>> in the *.x file, e.g.:
>>
>> %#define _msgout my_msgout
>> %extern void my_msgout(char *);
>>
>> or perhaps
>>
>> %#define _msgout(s)printf("RPC: %s\n", (s))
>>
>> I could make this change as a consolidation-private interface, but it
>> seems like it's of more general interest, suggesting an ARC case. At
>> that point it seems like I should involve the owners... which is what
>> brings us here.
>>
>> Any thoughts?
>> ___
>> nfs-discuss mailing list
>> nfs-discuss at opensolaris.org
>
>
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
On Thu, Apr 30, 2009 at 10:53:50AM -0700, Jordan Brown wrote: > The vast majority of the changes are to make the file pass cstyle and lint. That became necessary only once it became necessary to not generate this file every time. It's NOT necessary to make code generators generate cstyle-clean code. > It seems like it would be straightforward to fix rpcgen to generate > cleaner code, so that the generated code would be as similar as possible to > the modified-and-checked-in code, to minimize the work involved in a > resync. That seems like a relatively non-controversial bit of work. (Any > disagreement?) I don't think you should go touch rpcgen to make it generate cstyle-clean code. > I did that work, and cleaned up my existing *_svc.c file to eliminate > spurious differences between it and the generated file, and found that I > only had one difference left, one difference that prevented me from just > dynamically generating the file (which would avoid the need to fix the nits > mentioned above, though I'd probably fix them anyway). The generated > *_svc.c file includes a message handling function _msgout that calls syslog > or fprintf(stderr, ...) depending on a couple of flags. My application > has its own logging infrastructure, so I don't want to have this one file > doing its own thing. Ah! Then we should interpose on syslog() or punt. > I'd like to add a feature to rpcgen to allow the user to customize message > handling, so that the generated file can be used directly and there is no > need to hand-tweak it and check the file into the repository. That's fine too, but if it's easier to punt or interpose on syslog() then do that. Nico --
[nfs-discuss] rpcgen, cstyle, lint, and usability of rpcgen-generated files
I'm told that you folks are one of the better places to ask about rpcgen.
Please feel free to direct me elsewhere or forward as appropriate.
I've inherited a program that uses RPC.
For unknown historical reasons, its *_svc.c file has been modified and
checked into the repository. This is a nuisance because when I change the
*.x file and regenerate the *_svc.c file, I have to hand-merge the changes.
The vast majority of the changes are to make the file pass cstyle and lint.
It seems like it would be straightforward to fix rpcgen to generate
cleaner code, so that the generated code would be as similar as possible to
the modified-and-checked-in code, to minimize the work involved in a
resync. That seems like a relatively non-controversial bit of work. (Any
disagreement?)
I did that work, and cleaned up my existing *_svc.c file to eliminate
spurious differences between it and the generated file, and found that I
only had one difference left, one difference that prevented me from just
dynamically generating the file (which would avoid the need to fix the nits
mentioned above, though I'd probably fix them anyway). The generated
*_svc.c file includes a message handling function _msgout that calls syslog
or fprintf(stderr, ...) depending on a couple of flags. My application
has its own logging infrastructure, so I don't want to have this one file
doing its own thing.
I'd like to add a feature to rpcgen to allow the user to customize message
handling, so that the generated file can be used directly and there is no
need to hand-tweak it and check the file into the repository.
The change that I have prototyped is to surround the definition of _msgout with
#if !defined(_msgout)
...
#endif
This then allows one to supply an alternate definition using % lines in the
*.x file, e.g.:
%#define _msgout my_msgout
%extern void my_msgout(char *);
or perhaps
%#define _msgout(s)printf("RPC: %s\n", (s))
I could make this change as a consolidation-private interface, but it seems
like it's of more general interest, suggesting an ARC case. At that point
it seems like I should involve the owners... which is what brings us here.
Any thoughts?
