Re: fdesign -convert is buggy
Angus Leeming wrote:
I believe that we achieve what you're looking for with
-} else
- strcpy(fname, filename);
+} else {
+ size_t const buffer_size = 512;
+ char buffer[buffer_size];
+ char * cwd = getcwd(buffer, buffer_size);
+ if (cwd) {
+ strcpy(fname, cwd);
+ strcat(fname, /);
+ strcat(fname, filename_only(filename));
+ } else
+ strcpy(fname, filename);
+}
Test code attached. I'm pretty sure that this is safe, but would value
second opinions...
IMHO you should use strncat and strncpy instead of strcat and strcpy. I am
not sure if strncat requires it, but strncpy does not terminate the string
correctly if it was too long, so
fname[SIZE-1] = '\0';
is needed afterwards. This way too long names are just cut, otherwise you
may get buffer overflows. But I see that the original code does use strcpy
and strcat too, so it won't help much unless that code is converted too.
String manipulation in C is so nice ;-)
Georg
Re: fdesign -convert is buggy
Georg Baum wrote: Test code attached. I'm pretty sure that this is safe, but would value second opinions... IMHO you should use strncat and strncpy instead of strcat and strcpy. I am not sure if strncat requires it, but strncpy does not terminate the string correctly if it was too long, so fname[SIZE-1] = '\0'; is needed afterwards. This way too long names are just cut, otherwise you may get buffer overflows. But I see that the original code does use strcpy and strcat too, so it won't help much unless that code is converted too. String manipulation in C is so nice ;-) Isn't it just. Thanks for this, Georg. -- Angus
Re: fdesign -convert is buggy
Angus Leeming wrote:
String manipulation in C is so nice ;-)
Isn't it just. Thanks for this, Georg.
Actually, once you've got your head around the fact that nobody is
there to hold your hand, it isn't too bad. The attached trial program
has been tested and is always safe. I'm going to commit this to
XForms cvs.
--
Angus#include stdio.h
#include stdlib.h
#include limits.h
#include string.h
typedef struct {
char * output_dir;
} FD_Opt;
FD_Opt fdopt;
static char const *
filename_only(char const * filename)
{
char const * ptr = strrchr(filename, '/');
if (ptr)
return ptr+1;
return filename;
}
static void
build_fname(char * fname, size_t fname_capacity,
char const * filename, char const * ext)
{
char const * const only_filename = filename_only(filename);
size_t fname_size;
fname[fname_capacity] = '\0';
if (fdopt.output_dir) {
strncpy(fname, fdopt.output_dir, fname_capacity);
fname_size = strlen(fname);
if (fname_size != fname_capacity fname[fname_size - 1] != '/') {
strcat(fname, /);
fname_size += 1;
}
strncat(fname, only_filename, fname_capacity-fname_size);
} else
strncpy(fname, only_filename, fname_capacity);
fname_size = strlen(fname);
strncat(fname, ext, fname_capacity-fname_size);
}
int main(int argc, char * argv[])
{
size_t const fname_capacity = 10;
char fname[fname_capacity + 1];
fdopt.output_dir = 0;
if (argc 1) {
fdopt.output_dir = (char *)malloc(strlen(argv[1]) + 1);
strcpy(fdopt.output_dir, argv[1]);
}
build_fname(fname, fname_capacity, foo/bar/bazabcde, .h);
printf(Output fname is \%s\\n, fname);
return 0;
}
Re: fdesign -convert is buggy
Angus Leeming wrote:
> I believe that we achieve what you're looking for with
> -} else
> - strcpy(fname, filename);
> +} else {
> + size_t const buffer_size = 512;
> + char buffer[buffer_size];
> + char * cwd = getcwd(buffer, buffer_size);
> + if (cwd) {
> + strcpy(fname, cwd);
> + strcat(fname, "/");
> + strcat(fname, filename_only(filename));
> + } else
> + strcpy(fname, filename);
> +}
>
> Test code attached. I'm pretty sure that this is safe, but would value
> second opinions...
IMHO you should use strncat and strncpy instead of strcat and strcpy. I am
not sure if strncat requires it, but strncpy does not terminate the string
correctly if it was too long, so
fname[SIZE-1] = '\0';
is needed afterwards. This way too long names are just cut, otherwise you
may get buffer overflows. But I see that the original code does use strcpy
and strcat too, so it won't help much unless that code is converted too.
String manipulation in C is so nice ;-)
Georg
Re: fdesign -convert is buggy
Georg Baum wrote: >> Test code attached. I'm pretty sure that this is safe, but would >> value second opinions... > > IMHO you should use strncat and strncpy instead of strcat and > strcpy. I am not sure if strncat requires it, but strncpy does not > terminate the string correctly if it was too long, so > > fname[SIZE-1] = '\0'; > > is needed afterwards. This way too long names are just cut, > otherwise you may get buffer overflows. But I see that the original > code does use strcpy and strcat too, so it won't help much unless > that code is converted too. > > String manipulation in C is so nice ;-) Isn't it just. Thanks for this, Georg. -- Angus
Re: fdesign -convert is buggy
Angus Leeming wrote:
>> String manipulation in C is so nice ;-)
>
> Isn't it just. Thanks for this, Georg.
Actually, once you've got your head around the fact that nobody is
there to hold your hand, it isn't too bad. The attached trial program
has been tested and is always safe. I'm going to commit this to
XForms cvs.
--
Angus#include
#include
#include
#include
typedef struct {
char * output_dir;
} FD_Opt;
FD_Opt fdopt;
static char const *
filename_only(char const * filename)
{
char const * ptr = strrchr(filename, '/');
if (ptr)
return ptr+1;
return filename;
}
static void
build_fname(char * fname, size_t fname_capacity,
char const * filename, char const * ext)
{
char const * const only_filename = filename_only(filename);
size_t fname_size;
fname[fname_capacity] = '\0';
if (fdopt.output_dir) {
strncpy(fname, fdopt.output_dir, fname_capacity);
fname_size = strlen(fname);
if (fname_size != fname_capacity && fname[fname_size - 1] != '/') {
strcat(fname, "/");
fname_size += 1;
}
strncat(fname, only_filename, fname_capacity-fname_size);
} else
strncpy(fname, only_filename, fname_capacity);
fname_size = strlen(fname);
strncat(fname, ext, fname_capacity-fname_size);
}
int main(int argc, char * argv[])
{
size_t const fname_capacity = 10;
char fname[fname_capacity + 1];
fdopt.output_dir = 0;
if (argc > 1) {
fdopt.output_dir = (char *)malloc(strlen(argv[1]) + 1);
strcpy(fdopt.output_dir, argv[1]);
}
build_fname(fname, fname_capacity, "foo/bar/bazabcde", ".h");
printf("Output fname is \"%s\"\n", fname);
return 0;
}
Re: fdesign -convert is buggy
[EMAIL PROTECTED] (Lars Gullik Bjønnes) writes: | It seems that fdesign returns 0 even if something went wrong. | (writing the files f.ex.) And the arguemnt to convert _must_ be _just_ a file. Not even ./ is allowed in front of the filename. -- Lgb
Re: fdesign -convert is buggy
Lars Gullik Bjønnes wrote:
It seems that fdesign returns 0 even if something went wrong.
(writing the files f.ex.)
Wrong list...
In general, you're wrong. fdesign will exit early and return 1 in lots
of places:
if (!(fd_display = fl_initialize(ac, av, 0, fd_cmdopt, Ncopt)))
exit(1);
Also, it calls 'usage' in lots of places. the '1' leads to exit(1).
if (help)
usage(av[0], 1);
But in this specific conversion case you're right, fdesign always
returns 0:
if (fdopt.conv_only)
{
if (ac == 1)
fprintf(stderr, %s: -convert requires arguments\n, av[0]);
for (s = 1; s ac; s++)
{
load_forms(FALSE, av[s], 0);
save_forms(av[s]);
}
exit(0);
}
Both load_forms and save_forms return whether they were successful or
not, so this could easily be changed to:
for (s = 1; s ac; s++)
{
if (!load_forms(FALSE, av[s], 0))
exit(1);
if (!save_forms(av[s]))
exit(1);
}
Thanks for the heads up.
--
Angus
Re: fdesign -convert is buggy
Lars Gullik Bjønnes wrote: And the arguemnt to convert _must_ be _just_ a file. Not even ./ is allowed in front of the filename. I know. Crappy isn't it. -- Angus
Re: fdesign -convert is buggy
Angus Leeming [EMAIL PROTECTED] writes:
| Lars Gullik Bjønnes wrote:
It seems that fdesign returns 0 even if something went wrong.
I forgot to say when using -convert
| Wrong list...
I know... but this is where I have the problem...
| Both load_forms and save_forms return whether they were successful or
| not, so this could easily be changed to:
| for (s = 1; s ac; s++)
| {
| if (!load_forms(FALSE, av[s], 0))
| exit(1);
| if (!save_forms(av[s]))
| exit(1);
| }
| Thanks for the heads up.
Sure.
But some kind of error handling and a message to the user would be
nice.
And that it failes when the fd file is called as ./foo.fd is a bit
strange...
--
Lgb
Re: fdesign -convert is buggy
Lars Gullik Bjnnes wrote:
Angus Leeming [EMAIL PROTECTED] writes:
| Lars Gullik Bjnnes wrote:
It seems that fdesign returns 0 even if something went wrong.
I forgot to say when using -convert
| Wrong list...
I know... but this is where I have the problem...
| Both load_forms and save_forms return whether they were successful
| or not, so this could easily be changed to:
| for (s = 1; s ac; s++)
| {
| if (!load_forms(FALSE, av[s], 0))
| exit(1);
| if (!save_forms(av[s]))
| exit(1);
| }
| Thanks for the heads up.
Sure.
But some kind of error handling and a message to the user would be
nice.
And that it failes when the fd file is called as ./foo.fd is a bit
strange...
Do you have xforms cvs? If my testing doesn't throw up any problems,
I'm going to commit the attached patch. It gives you improved
diagnostics.
The cvs version of fdesign will already enable you to run
$ fdesign -dir destdir -convert srcdir/form_aboutlyx.fd
For example:
Testing shows that running the cvs version of fdesign from the build
dir (!= src dir) so:
fdesign -convert
/home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd
works, shoving form_external.[ch] in the same directory as the .fd
file.
Moreover, running (again from the builddir != src dir):
fdesign -dir . -convert
/home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd
shoves form_external.[ch] in the build dir.
Ie, current cvs does all that you want of it.
--
AngusIndex: fdesign/fd_main.c
===
RCS file: /cvsroot/xforms/xforms/fdesign/fd_main.c,v
retrieving revision 1.10
diff -u -p -r1.10 fd_main.c
--- fdesign/fd_main.c 18 May 2004 13:57:11 - 1.10
+++ fdesign/fd_main.c 27 May 2004 12:09:10 -
@@ -86,12 +86,10 @@ FD_Opt fdopt;
Conv convertor[MAX_CONVERTOR + 1];
int fd_cntlborder;
-int help; /* if convert only */
int fd_bwidth;
int is_pasting;
int fd_trackgeometry = 1;
int fd_show_palette;
-int fd_pversion;
int fd_buttonLabelSize;
int fd_helpfontsize = 14;
int fd_align_fontsize = FL_TINY_SIZE;
@@ -587,15 +585,7 @@ pre_connect(int ac, char *av[])
if (fdopt.conv_only)
{
- fl_set_app_name(av[0], Fdesign); /* resource routine want this
-
-
-
-
-
-
-
- */
+ fl_set_app_name(av[0], Fdesign); /* resource routine wants this */
fl_init_fl_context();
create_the_forms();
init_classes();
@@ -610,8 +600,14 @@ pre_connect(int ac, char *av[])
for (s = i; s ac; s++)
{
- if (load_forms(FALSE, av[s], 0) = 0)
- save_forms(av[s]);
+ if (load_forms(FALSE, av[s], 0) 0) {
+ fprintf(stderr, Unable to load %s\n, av[s]);
+ exit(1);
+ }
+ if (!save_forms(av[s])) {
+ fprintf(stderr, Unable to convert %s\n, av[s]);
+ exit(1);
+ }
}
exit(0);
}
@@ -721,7 +717,9 @@ main(int ac, char *av[])
initialize();
-/* For conversion and usage help, we don't need a connection */
+/* For conversion, version and usage help, we don't need a connection.
+ pre_connect will exit in such circumstances.
+*/
pre_connect(ac, av);
/* force fdesign to come up in default visual */
@@ -741,19 +739,12 @@ main(int ac, char *av[])
fl_get_app_resources(fdres, Nropt);
fl_add_signal_callback(SIGINT, interrupted, 0);
-if (fd_pversion)
- print_version(1);
-
-if (help)
- usage(av[0], 1);
-
if (av[1] av[1][0] == '-')
{
fprintf(stderr, Unknown option: %s\n, av[1]);
usage(av[0], 1);
}
-
fl_cntl.coordUnit = FL_COORD_PIXEL;
fdopt.unit = unit_val(fd_sunit);
fdopt.language = lang_val(fd_slanguage);
@@ -780,22 +771,8 @@ main(int ac, char *av[])
fd_buttonLabelSize = fl_cntl.buttonFontSize;
fl_cntl.buttonFontSize = 0;
-
/* Initialize stuff */
init_classes();
-
-if (fdopt.conv_only)
-{
- if (ac == 1)
- fprintf(stderr, %s: -convert requires arguments\n, av[0]);
-
- for (s = 1; s ac; s++)
- {
- load_forms(FALSE, av[s], 0);
- save_forms(av[s]);
- }
- exit(0);
-}
fl_set_counter_bounds(fd_align-snapobj, 0.0, 500.0);
fl_set_counter_step(fd_align-snapobj, 1.0, 5.0);
Re: fdesign -convert is buggy
Angus Leeming [EMAIL PROTECTED] writes: | Do you have xforms cvs? no (and that is probably good since I now caught the problem myself, and didn't have to debug somebody elses problem) | If my testing doesn't throw up any problems, | I'm going to commit the attached patch. It gives you improved | diagnostics. nice | The cvs version of fdesign will already enable you to run | $ fdesign -dir destdir -convert srcdir/form_aboutlyx.fd | For example: | Testing shows that running the cvs version of fdesign from the build | dir (!= src dir) so: | fdesign -convert | /home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd | works, shoving form_external.[ch] in the same directory as the .fd | file. I don't agree with that behaviour. | Moreover, running (again from the builddir != src dir): | fdesign -dir . -convert | /home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd is the -dir . needed? IMHO it shouldn't be. fdesign should just output its files to PWD | shoves form_external.[ch] in the build dir. | Ie, current cvs does all that you want of it. Good. But until we require xforms 1.1 we will need the work-around. -- Lgb
Re: fdesign -convert is buggy
Angus Leeming [EMAIL PROTECTED] writes:
| @@ -610,8 +600,14 @@ pre_connect(int ac, char *av[])
|
| for (s = i; s ac; s++)
| {
| - if (load_forms(FALSE, av[s], 0) = 0)
| - save_forms(av[s]);
| + if (load_forms(FALSE, av[s], 0) 0) {
| + fprintf(stderr, Unable to load %s\n, av[s]);
| + exit(1);
| + }
| + if (!save_forms(av[s])) {
| + fprintf(stderr, Unable to convert %s\n, av[s]);
| + exit(1);
| + }
What if save_forms fails becuase it is unable to create the new files?
But conversion proper when just ok?
| @@ -780,22 +771,8 @@ main(int ac, char *av[])
| fd_buttonLabelSize = fl_cntl.buttonFontSize;
| fl_cntl.buttonFontSize = 0;
|
| -
| /* Initialize stuff */
| init_classes();
| -
| -if (fdopt.conv_only)
| -{
| - if (ac == 1)
| - fprintf(stderr, %s: -convert requires arguments\n, av[0]);
| -
| - for (s = 1; s ac; s++)
| - {
| - load_forms(FALSE, av[s], 0);
| - save_forms(av[s]);
| - }
| - exit(0);
| -}
Isn't this removing functionality?
--
Lgb
Re: fdesign -convert is buggy
Lars == Lars Gullik Bjønnes [EMAIL PROTECTED] writes: Lars | Moreover, running (again from the builddir != src dir): Lars | fdesign -dir . -convert | Lars /home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd Lars is the -dir . needed? IMHO it shouldn't be. fdesign should just Lars output its files to PWD This would break compatibility with older versions of fdesign. JMarc
Re: fdesign -convert is buggy
Jean-Marc Lasgouttes [EMAIL PROTECTED] writes: Lars == Lars Gullik Bjønnes [EMAIL PROTECTED] writes: | Lars | Moreover, running (again from the builddir != src dir): | Lars | fdesign -dir . -convert | | Lars /home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd | Lars is the -dir . needed? IMHO it shouldn't be. fdesign should just | Lars output its files to PWD | This would break compatibility with older versions of fdesign. Would it really? Older versions of fdesign only works with the *.fd file in the same dir as where fdesign is run... So I see no compatibility problems. -- Lgb
Re: fdesign -convert is buggy
Lars == Lars Gullik Bjønnes [EMAIL PROTECTED] writes: Lars Jean-Marc Lasgouttes [EMAIL PROTECTED] writes: Lars | This would break compatibility with older versions of fdesign. Lars Would it really? Lars Older versions of fdesign only works with the *.fd file in the Lars same dir as where fdesign is run... Lars So I see no compatibility problems. Good remark, it seems you are right. Angus, this probably means that we can/should get rid of the -dir argument. JMarc
Re: fdesign -convert is buggy
Lars Gullik Bjønnes wrote:
Angus Leeming [EMAIL PROTECTED] writes:
| @@ -610,8 +600,14 @@ pre_connect(int ac, char *av[])
|
| for (s = i; s ac; s++)
| {
| - if (load_forms(FALSE, av[s], 0) = 0)
| - save_forms(av[s]);
| + if (load_forms(FALSE, av[s], 0) 0) {
| + fprintf(stderr, Unable to load %s\n, av[s]);
| + exit(1);
| + }
| + if (!save_forms(av[s])) {
| + fprintf(stderr, Unable to convert %s\n, av[s]);
| + exit(1);
| + }
What if save_forms fails becuase it is unable to create the new
files? But conversion proper when just ok?
The routine is misnamed. It does both (any necessary) saving and
conversion.
| @@ -780,22 +771,8 @@ main(int ac, char *av[])
| fd_buttonLabelSize = fl_cntl.buttonFontSize;
| fl_cntl.buttonFontSize = 0;
|
| -
| /* Initialize stuff */
| init_classes();
| -
| -if (fdopt.conv_only)
| -{
| - if (ac == 1)
| - fprintf(stderr, %s: -convert requires arguments\n, av[0]);
| -
| - for (s = 1; s ac; s++)
| - {
| - load_forms(FALSE, av[s], 0);
| - save_forms(av[s]);
| - }
| - exit(0);
| -}
Isn't this removing functionality?
No. This code is never reached.
--
Angus
Re: fdesign -convert is buggy
Jean-Marc Lasgouttes wrote: Lars Jean-Marc Lasgouttes [EMAIL PROTECTED] writes: Lars | This would break compatibility with older versions of fdesign. Lars Would it really? Lars Older versions of fdesign only works with the *.fd file in the Lars same dir as where fdesign is run... Lars So I see no compatibility problems. Good remark, it seems you are right. Angus, this probably means that we can/should get rid of the -dir argument. We would merely be swapping one semantic that works for another one which is not yet proven. I don't see what it gains us. As I see it I can do: $ cd $HOME $ fdesign -dir destdir -convert foo/bar.fd which seems more flexible to me than Lars' proposal. -- Angus
Re: fdesign -convert is buggy
Angus == Angus Leeming [EMAIL PROTECTED] writes: Angus We would merely be swapping one semantic that works for another Angus one which is not yet proven. I don't see what it gains us. As I Angus see it I can do: Angus $ cd $HOME $ fdesign -dir destdir -convert foo/bar.fd Angus which seems more flexible to me than Lars' proposal. What does uic do? JMarc
Re: fdesign -convert is buggy
Jean-Marc Lasgouttes wrote: Angus == Angus Leeming [EMAIL PROTECTED] writes: Angus We would merely be swapping one semantic that works for another Angus one which is not yet proven. I don't see what it gains us. As I Angus see it I can do: Angus $ cd $HOME $ fdesign -dir destdir -convert foo/bar.fd Angus which seems more flexible to me than Lars' proposal. What does uic do? JMarc 1. It produces only one file per invocation, either .h or .C: Generate declaration: uic [options] file Generate implementation: uic [options] -impl headerfile file headerfile:name of the declaration file fdesign produces both .h and .c file in the same step. GENERAL OPTIONS -o file Write output to file rather than to stdout. Man page attached, but my conclusion is that uic is just different. Nonetheless, the sematics that we have now have similar power to those of uic. -- Angus uic.1.gz Description: GNU Zip compressed data
Re: fdesign -convert is buggy
Angus Leeming [EMAIL PROTECTED] writes: | Man page attached, but my conclusion is that uic is just different. | Nonetheless, the sematics that we have now have similar power to | those of uic. Except for the surprising behaviour that fdesign output files to the same dir there the .fd file is located. Sure, keep -dir, but make fdesign output to pwd not to the .fd files dir. -- Lgb
Re: fdesign -convert is buggy
Angus Leeming [EMAIL PROTECTED] writes: | Jean-Marc Lasgouttes wrote: Lars Jean-Marc Lasgouttes [EMAIL PROTECTED] writes: Lars | This would break compatibility with older versions of fdesign. Lars Would it really? Lars Older versions of fdesign only works with the *.fd file in the Lars same dir as where fdesign is run... Lars So I see no compatibility problems. Good remark, it seems you are right. Angus, this probably means that we can/should get rid of the -dir argument. | We would merely be swapping one semantic that works for another one | which is not yet proven. I don't see what it gains us. As I see it I | can do: Note that the old semantic is kept as well. Only if you do something that was not possible with old fdesign the semantics change. | $ cd $HOME | $ fdesign -dir destdir -convert foo/bar.fd | which seems more flexible to me than Lars' proposal. surely not. (and I am not saying that -dir should be removed, I am saying that fdesign should output to pwd (unless -dir is specified of course.)) -- Lgb
Re: fdesign -convert is buggy
Lars Gullik Bjønnes wrote: (and I am not saying that -dir should be removed, I am saying that fdesign should output to pwd (unless -dir is specified of course.)) Ok, understood. You want me to mess around with malloc just before we try and get XForms 1.1 out of the door. Bad man! I'll have a look. -- Angus
Re: fdesign -convert is buggy
Angus Leeming wrote:
Lars Gullik Bjønnes wrote:
(and I am not saying that -dir should be removed, I am saying that
fdesign should output to pwd (unless -dir is specified of course.))
Ok, understood. You want me to mess around with malloc just before
we try and get XForms 1.1 out of the door. Bad man!
I'll have a look.
This is the current code.
static char const *
filename_only(char const * filename)
{
char const * ptr = strrchr(filename, '/');
if (ptr)
return ptr+1;
return filename;
}
/* Given 'filename' (no extension) and 'ext', generate 'fname' */
static void
build_fname(char * fname, char const * filename, char const * ext)
{
if (fdopt.output_dir) {
strcpy(fname, fdopt.output_dir);
if (fname[strlen(fdopt.output_dir) - 1] != '/')
strcat(fname, /);
strcat(fname, filename_only(filename));
} else
strcpy(fname, filename);
strcat(fname, ext);
}
I believe that we achieve what you're looking for with
-} else
- strcpy(fname, filename);
+} else {
+ size_t const buffer_size = 512;
+ char buffer[buffer_size];
+ char * cwd = getcwd(buffer, buffer_size);
+ if (cwd) {
+ strcpy(fname, cwd);
+ strcat(fname, /);
+ strcat(fname, filename_only(filename));
+ } else
+ strcpy(fname, filename);
+}
Test code attached. I'm pretty sure that this is safe, but would value
second opinions...
--
Angus#include stdio.h
#include string.h
#include unistd.h
typedef struct {
char * output_dir;
} FD_Opt;
FD_Opt fdopt;
static char const *
filename_only(char const * filename)
{
char const * ptr = strrchr(filename, '/');
if (ptr)
return ptr+1;
return filename;
}
/* Given 'filename' (no extension) and 'ext', generate 'fname' */
static void
build_fname(char * fname, char const * filename, char const * ext)
{
if (fdopt.output_dir) {
strcpy(fname, fdopt.output_dir);
if (fname[strlen(fdopt.output_dir) - 1] != '/')
strcat(fname, /);
strcat(fname, filename_only(filename));
} else {
size_t const buffer_size = 512;
char buffer[buffer_size];
char * cwd = getcwd(buffer, buffer_size);
if (cwd) {
strcpy(fname, cwd);
strcat(fname, /);
strcat(fname, filename_only(filename));
} else
strcpy(fname, filename);
}
strcat(fname, ext);
}
int main(int argc, char * argv[])
{
char fname[512];
fdopt.output_dir = 0;
if (argc 1) {
fdopt.output_dir = (char *)malloc(strlen(argv[1]) + 1);
strcpy(fdopt.output_dir, argv[1]);
}
build_fname(fname, foo/bar/baz, .h);
printf(Output fname is \%s\\n, fname);
return 0;
}
Re: fdesign -convert is buggy
Angus Leeming [EMAIL PROTECTED] writes:
| I believe that we achieve what you're looking for with
| -} else
| - strcpy(fname, filename);
| +} else {
| + size_t const buffer_size = 512;
| + char buffer[buffer_size];
| + char * cwd = getcwd(buffer, buffer_size);
| + if (cwd) {
| + strcpy(fname, cwd);
| + strcat(fname, /);
| + strcat(fname, filename_only(filename));
| + } else
| + strcpy(fname, filename);
Why isn't filename_only used here?
| static char const *
| filename_only(char const * filename)
| {
| char const * ptr = strrchr(filename, '/');
| if (ptr)
| return ptr+1;
| return filename;
| }
| /* Given 'filename' (no extension) and 'ext', generate 'fname' */
| static void
| build_fname(char * fname, char const * filename, char const * ext)
| {
| if (fdopt.output_dir) {
| strcpy(fname, fdopt.output_dir);
| if (fname[strlen(fdopt.output_dir) - 1] != '/')
| strcat(fname, /);
| strcat(fname, filename_only(filename));
| } else {
| size_t const buffer_size = 512;
| char buffer[buffer_size];
| char * cwd = getcwd(buffer, buffer_size);
| if (cwd) {
| strcpy(fname, cwd);
| strcat(fname, /);
| strcat(fname, filename_only(filename));
| } else
| strcpy(fname, filename);
| }
| strcat(fname, ext);
| }
It almost seems to me that the else clause should just be
strcpy(fname, filename_only(filename));
and that you don't have to fiddle with getcwd at all?
| int main(int argc, char * argv[])
| {
| char fname[512];
Is this big enough?
(I have no idea what the actual value in xforms is, but PATH_MAX is a
nice value)
| fdopt.output_dir = 0;
| if (argc 1) {
| fdopt.output_dir = (char *)malloc(strlen(argv[1]) + 1);
| strcpy(fdopt.output_dir, argv[1]);
| }
| build_fname(fname, foo/bar/baz, .h);
If my above method was used this would produce just baz.h. but the old
way would produce foo/bar/baz.h
--
Lgb
Re: fdesign -convert is buggy
Lars Gullik Bjønnes wrote:
| } else
| strcpy(fname, filename);
| }
| strcat(fname, ext);
| }
It almost seems to me that the else clause should just be
strcpy(fname, filename_only(filename));
I think you're correct. Thanks.
| build_fname(fname, foo/bar/baz, .h);
If my above method was used this would produce just baz.h. but the
old way would produce foo/bar/baz.h
Correct.
The test program seems to work exactly as you want.
Jean-Marc, are you happy with the change?
Index: fdesign/fd_printC.c
===
RCS file: /cvsroot/xforms/xforms/fdesign/fd_printC.c,v
retrieving revision 1.8
diff -u -p -r1.8 fd_printC.c
--- fdesign/fd_printC.c 27 Nov 2003 18:15:00 - 1.8
+++ fdesign/fd_printC.c 27 May 2004 16:50:46 -
@@ -82,7 +82,7 @@ build_fname(char * fname, char const * f
strcat(fname, /);
strcat(fname, filename_only(filename));
} else
- strcpy(fname, filename);
+ strcpy(fname, filename_only(filename));
strcat(fname, ext);
}
--
Angus#include stdio.h
#include limits.h
#include string.h
typedef struct {
char * output_dir;
} FD_Opt;
FD_Opt fdopt;
static char const *
filename_only(char const * filename)
{
char const * ptr = strrchr(filename, '/');
if (ptr)
return ptr+1;
return filename;
}
/* Given 'filename' (no extension) and 'ext', generate 'fname' */
static void
build_fname(char * fname, char const * filename, char const * ext)
{
if (fdopt.output_dir) {
strcpy(fname, fdopt.output_dir);
if (fname[strlen(fdopt.output_dir) - 1] != '/')
strcat(fname, /);
strcat(fname, filename_only(filename));
} else
strcpy(fname, filename_only(filename));
strcat(fname, ext);
}
int main(int argc, char * argv[])
{
char fname[PATH_MAX + 1];
fdopt.output_dir = 0;
if (argc 1) {
fdopt.output_dir = (char *)malloc(strlen(argv[1]) + 1);
strcpy(fdopt.output_dir, argv[1]);
}
build_fname(fname, foo/bar/baz, .h);
printf(Output fname is \%s\\n, fname);
return 0;
}
Re: fdesign -convert is buggy
[EMAIL PROTECTED] (Lars Gullik Bjønnes) writes: | It seems that fdesign returns 0 even if something went wrong. > | (writing the files f.ex.) And the arguemnt to convert _must_ be _just_ a file. Not even ./ is allowed in front of the filename. -- Lgb
Re: fdesign -convert is buggy
Lars Gullik Bjønnes wrote:
> It seems that fdesign returns 0 even if something went wrong.
>
> (writing the files f.ex.)
Wrong list...
In general, you're wrong. fdesign will exit early and return 1 in lots
of places:
if (!(fd_display = fl_initialize(, av, 0, fd_cmdopt, Ncopt)))
exit(1);
Also, it calls 'usage' in lots of places. the '1' leads to exit(1).
if (help)
usage(av[0], 1);
But in this specific conversion case you're right, fdesign always
returns 0:
if (fdopt.conv_only)
{
if (ac == 1)
fprintf(stderr, "%s: -convert requires arguments\n", av[0]);
for (s = 1; s < ac; s++)
{
load_forms(FALSE, av[s], 0);
save_forms(av[s]);
}
exit(0);
}
Both load_forms and save_forms return whether they were successful or
not, so this could easily be changed to:
for (s = 1; s < ac; s++)
{
if (!load_forms(FALSE, av[s], 0))
exit(1);
if (!save_forms(av[s]))
exit(1);
}
Thanks for the heads up.
--
Angus
Re: fdesign -convert is buggy
Lars Gullik Bjønnes wrote: > And the arguemnt to convert _must_ be _just_ a file. Not even ./ is > allowed in front of the filename. I know. Crappy isn't it. -- Angus
Re: fdesign -convert is buggy
Angus Leeming <[EMAIL PROTECTED]> writes:
| Lars Gullik Bjønnes wrote:
>> It seems that fdesign returns 0 even if something went wrong.
I forgot to say "when using -convert"
| Wrong list...
I know... but this is where I have the problem...
| Both load_forms and save_forms return whether they were successful or
| not, so this could easily be changed to:
>
| for (s = 1; s < ac; s++)
| {
| if (!load_forms(FALSE, av[s], 0))
| exit(1);
| if (!save_forms(av[s]))
| exit(1);
| }
>
| Thanks for the heads up.
Sure.
But some kind of error handling and a message to the user would be
nice.
And that it failes when the fd file is called as ./foo.fd is a bit
strange...
--
Lgb
Re: fdesign -convert is buggy
Lars Gullik Bjnnes wrote:
> Angus Leeming <[EMAIL PROTECTED]> writes:
>
> | Lars Gullik Bjnnes wrote:
>>> It seems that fdesign returns 0 even if something went wrong.
>
> I forgot to say "when using -convert"
>
> | Wrong list...
>
> I know... but this is where I have the problem...
>
> | Both load_forms and save_forms return whether they were successful
> | or not, so this could easily be changed to:
>>
> | for (s = 1; s < ac; s++)
> | {
> | if (!load_forms(FALSE, av[s], 0))
> | exit(1);
> | if (!save_forms(av[s]))
> | exit(1);
> | }
>>
> | Thanks for the heads up.
>
> Sure.
>
> But some kind of error handling and a message to the user would be
> nice.
>
> And that it failes when the fd file is called as ./foo.fd is a bit
> strange...
Do you have xforms cvs? If my testing doesn't throw up any problems,
I'm going to commit the attached patch. It gives you improved
diagnostics.
The cvs version of fdesign will already enable you to run
$ fdesign -dir -convert form_aboutlyx.fd
For example:
Testing shows that running the cvs version of fdesign from the build
dir (!= src dir) so:
fdesign -convert
/home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd
works, shoving form_external.[ch] in the same directory as the .fd
file.
Moreover, running (again from the builddir != src dir):
fdesign -dir . -convert
/home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd
shoves form_external.[ch] in the build dir.
Ie, current cvs does all that you want of it.
--
AngusIndex: fdesign/fd_main.c
===
RCS file: /cvsroot/xforms/xforms/fdesign/fd_main.c,v
retrieving revision 1.10
diff -u -p -r1.10 fd_main.c
--- fdesign/fd_main.c 18 May 2004 13:57:11 - 1.10
+++ fdesign/fd_main.c 27 May 2004 12:09:10 -
@@ -86,12 +86,10 @@ FD_Opt fdopt;
Conv convertor[MAX_CONVERTOR + 1];
int fd_cntlborder;
-int help; /* if convert only */
int fd_bwidth;
int is_pasting;
int fd_trackgeometry = 1;
int fd_show_palette;
-int fd_pversion;
int fd_buttonLabelSize;
int fd_helpfontsize = 14;
int fd_align_fontsize = FL_TINY_SIZE;
@@ -587,15 +585,7 @@ pre_connect(int ac, char *av[])
if (fdopt.conv_only)
{
- fl_set_app_name(av[0], "Fdesign"); /* resource routine want this
-
-
-
-
-
-
-
- */
+ fl_set_app_name(av[0], "Fdesign"); /* resource routine wants this */
fl_init_fl_context();
create_the_forms();
init_classes();
@@ -610,8 +600,14 @@ pre_connect(int ac, char *av[])
for (s = i; s < ac; s++)
{
- if (load_forms(FALSE, av[s], 0) >= 0)
- save_forms(av[s]);
+ if (load_forms(FALSE, av[s], 0) < 0) {
+ fprintf(stderr, "Unable to load %s\n", av[s]);
+ exit(1);
+ }
+ if (!save_forms(av[s])) {
+ fprintf(stderr, "Unable to convert %s\n", av[s]);
+ exit(1);
+ }
}
exit(0);
}
@@ -721,7 +717,9 @@ main(int ac, char *av[])
initialize();
-/* For conversion and usage help, we don't need a connection */
+/* For conversion, version and usage help, we don't need a connection.
+ pre_connect will exit in such circumstances.
+*/
pre_connect(ac, av);
/* force fdesign to come up in default visual */
@@ -741,19 +739,12 @@ main(int ac, char *av[])
fl_get_app_resources(fdres, Nropt);
fl_add_signal_callback(SIGINT, interrupted, 0);
-if (fd_pversion)
- print_version(1);
-
-if (help)
- usage(av[0], 1);
-
if (av[1] && av[1][0] == '-')
{
fprintf(stderr, " Unknown option: %s\n", av[1]);
usage(av[0], 1);
}
-
fl_cntl.coordUnit = FL_COORD_PIXEL;
fdopt.unit = unit_val(fd_sunit);
fdopt.language = lang_val(fd_slanguage);
@@ -780,22 +771,8 @@ main(int ac, char *av[])
fd_buttonLabelSize = fl_cntl.buttonFontSize;
fl_cntl.buttonFontSize = 0;
-
/* Initialize stuff */
init_classes();
-
-if (fdopt.conv_only)
-{
- if (ac == 1)
- fprintf(stderr, "%s: -convert requires arguments\n", av[0]);
-
- for (s = 1; s < ac; s++)
- {
- load_forms(FALSE, av[s], 0);
- save_forms(av[s]);
- }
- exit(0);
-}
fl_set_counter_bounds(fd_align->snapobj, 0.0, 500.0);
fl_set_counter_step(fd_align->snapobj, 1.0, 5.0);
Re: fdesign -convert is buggy
Angus Leeming <[EMAIL PROTECTED]> writes: | Do you have xforms cvs? no (and that is probably good since I now caught the problem myself, and didn't have to debug somebody elses problem) | If my testing doesn't throw up any problems, | I'm going to commit the attached patch. It gives you improved | diagnostics. nice > | The cvs version of fdesign will already enable you to run | $ fdesign -dir -convert form_aboutlyx.fd > | For example: > | Testing shows that running the cvs version of fdesign from the build | dir (!= src dir) so: | fdesign -convert | /home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd > | works, shoving form_external.[ch] in the same directory as the .fd | file. I don't agree with that behaviour. > | Moreover, running (again from the builddir != src dir): > | fdesign -dir . -convert | /home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd is the -dir . needed? IMHO it shouldn't be. fdesign should just output its files to PWD | shoves form_external.[ch] in the build dir. > | Ie, current cvs does all that you want of it. Good. But until we require xforms 1.1 we will need the work-around. -- Lgb
Re: fdesign -convert is buggy
Angus Leeming <[EMAIL PROTECTED]> writes:
| @@ -610,8 +600,14 @@ pre_connect(int ac, char *av[])
|
| for (s = i; s < ac; s++)
| {
| - if (load_forms(FALSE, av[s], 0) >= 0)
| - save_forms(av[s]);
| + if (load_forms(FALSE, av[s], 0) < 0) {
| + fprintf(stderr, "Unable to load %s\n", av[s]);
| + exit(1);
| + }
| + if (!save_forms(av[s])) {
| + fprintf(stderr, "Unable to convert %s\n", av[s]);
| + exit(1);
| + }
What if save_forms fails becuase it is unable to create the new files?
But conversion proper when just ok?
| @@ -780,22 +771,8 @@ main(int ac, char *av[])
| fd_buttonLabelSize = fl_cntl.buttonFontSize;
| fl_cntl.buttonFontSize = 0;
|
| -
| /* Initialize stuff */
| init_classes();
| -
| -if (fdopt.conv_only)
| -{
| - if (ac == 1)
| - fprintf(stderr, "%s: -convert requires arguments\n", av[0]);
| -
| - for (s = 1; s < ac; s++)
| - {
| - load_forms(FALSE, av[s], 0);
| - save_forms(av[s]);
| - }
| - exit(0);
| -}
Isn't this removing functionality?
--
Lgb
Re: fdesign -convert is buggy
> "Lars" == Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes: Lars> | Moreover, running (again from the builddir != src dir): >> Lars> | fdesign -dir . -convert | Lars> /home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd Lars> is the -dir . needed? IMHO it shouldn't be. fdesign should just Lars> output its files to PWD This would break compatibility with older versions of fdesign. JMarc
Re: fdesign -convert is buggy
Jean-Marc Lasgouttes <[EMAIL PROTECTED]> writes: >> "Lars" == Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes: > | Lars> | Moreover, running (again from the builddir != src dir): >>> | Lars> | fdesign -dir . -convert | | Lars> /home/angus/lyx/devel/src/frontends/xforms/forms/form_external.fd > | Lars> is the -dir . needed? IMHO it shouldn't be. fdesign should just | Lars> output its files to PWD > | This would break compatibility with older versions of fdesign. Would it really? Older versions of fdesign only works with the *.fd file in the same dir as where fdesign is run... So I see no compatibility problems. -- Lgb
Re: fdesign -convert is buggy
> "Lars" == Lars Gullik Bjønnes <[EMAIL PROTECTED]> writes: Lars> Jean-Marc Lasgouttes <[EMAIL PROTECTED]> writes: Lars> | This would break compatibility with older versions of fdesign. Lars> Would it really? Lars> Older versions of fdesign only works with the *.fd file in the Lars> same dir as where fdesign is run... Lars> So I see no compatibility problems. Good remark, it seems you are right. Angus, this probably means that we can/should get rid of the -dir argument. JMarc
Re: fdesign -convert is buggy
Lars Gullik Bjønnes wrote:
> Angus Leeming <[EMAIL PROTECTED]> writes:
>
> | @@ -610,8 +600,14 @@ pre_connect(int ac, char *av[])
> |
> | for (s = i; s < ac; s++)
> | {
> | - if (load_forms(FALSE, av[s], 0) >= 0)
> | - save_forms(av[s]);
> | + if (load_forms(FALSE, av[s], 0) < 0) {
> | + fprintf(stderr, "Unable to load %s\n", av[s]);
> | + exit(1);
> | + }
> | + if (!save_forms(av[s])) {
> | + fprintf(stderr, "Unable to convert %s\n", av[s]);
> | + exit(1);
> | + }
>
> What if save_forms fails becuase it is unable to create the new
> files? But conversion proper when just ok?
The routine is misnamed. It does both (any necessary) saving and
conversion.
>
> | @@ -780,22 +771,8 @@ main(int ac, char *av[])
> | fd_buttonLabelSize = fl_cntl.buttonFontSize;
> | fl_cntl.buttonFontSize = 0;
> |
> | -
> | /* Initialize stuff */
> | init_classes();
> | -
> | -if (fdopt.conv_only)
> | -{
> | - if (ac == 1)
> | - fprintf(stderr, "%s: -convert requires arguments\n", av[0]);
> | -
> | - for (s = 1; s < ac; s++)
> | - {
> | - load_forms(FALSE, av[s], 0);
> | - save_forms(av[s]);
> | - }
> | - exit(0);
> | -}
>
> Isn't this removing functionality?
No. This code is never reached.
--
Angus
Re: fdesign -convert is buggy
Jean-Marc Lasgouttes wrote: > Lars> Jean-Marc Lasgouttes <[EMAIL PROTECTED]> writes: > Lars> | This would break compatibility with older versions of > fdesign. > > Lars> Would it really? > > Lars> Older versions of fdesign only works with the *.fd file in the > Lars> same dir as where fdesign is run... > > Lars> So I see no compatibility problems. > > Good remark, it seems you are right. > > Angus, this probably means that we can/should get rid of the -dir > argument. We would merely be swapping one semantic that works for another one which is not yet proven. I don't see what it gains us. As I see it I can do: $ cd $HOME $ fdesign -dir destdir -convert foo/bar.fd which seems more flexible to me than Lars' proposal. -- Angus
Re: fdesign -convert is buggy
> "Angus" == Angus Leeming <[EMAIL PROTECTED]> writes: Angus> We would merely be swapping one semantic that works for another Angus> one which is not yet proven. I don't see what it gains us. As I Angus> see it I can do: Angus> $ cd $HOME $ fdesign -dir destdir -convert foo/bar.fd Angus> which seems more flexible to me than Lars' proposal. What does uic do? JMarc
Re: fdesign -convert is buggy
Jean-Marc Lasgouttes wrote: >> "Angus" == Angus Leeming <[EMAIL PROTECTED]> writes: > > Angus> We would merely be swapping one semantic that works for > another Angus> one which is not yet proven. I don't see what it > gains us. As I Angus> see it I can do: > > Angus> $ cd $HOME $ fdesign -dir destdir -convert foo/bar.fd > > Angus> which seems more flexible to me than Lars' proposal. > > What does uic do? > > JMarc 1. It produces only one file per invocation, either .h or .C: Generate declaration: uic [options] Generate implementation: uic [options] -impl :name of the declaration file fdesign produces both .h and .c file in the same step. GENERAL OPTIONS -o file Write output to file rather than to stdout. Man page attached, but my conclusion is that uic is just different. Nonetheless, the sematics that we have now have similar power to those of uic. -- Angus uic.1.gz Description: GNU Zip compressed data
Re: fdesign -convert is buggy
Angus Leeming <[EMAIL PROTECTED]> writes: | Man page attached, but my conclusion is that uic is just different. | Nonetheless, the sematics that we have now have similar power to | those of uic. Except for the surprising behaviour that fdesign output files to the same dir there the .fd file is located. Sure, keep -dir, but make fdesign output to pwd not to the .fd files dir. -- Lgb
Re: fdesign -convert is buggy
Angus Leeming <[EMAIL PROTECTED]> writes: | Jean-Marc Lasgouttes wrote: >> Lars> Jean-Marc Lasgouttes <[EMAIL PROTECTED]> writes: >> Lars> | This would break compatibility with older versions of >> fdesign. >> >> Lars> Would it really? >> >> Lars> Older versions of fdesign only works with the *.fd file in the >> Lars> same dir as where fdesign is run... >> >> Lars> So I see no compatibility problems. >> >> Good remark, it seems you are right. >> >> Angus, this probably means that we can/should get rid of the -dir >> argument. > | We would merely be swapping one semantic that works for another one | which is not yet proven. I don't see what it gains us. As I see it I | can do: Note that the old semantic is kept as well. Only if you do something that was not possible with old fdesign the semantics change. > | $ cd $HOME | $ fdesign -dir destdir -convert foo/bar.fd > | which seems more flexible to me than Lars' proposal. surely not. (and I am not saying that -dir should be removed, I am saying that fdesign should output to pwd (unless -dir is specified of course.)) -- Lgb
Re: fdesign -convert is buggy
Lars Gullik Bjønnes wrote: > (and I am not saying that -dir should be removed, I am saying that > fdesign should output to pwd (unless -dir is specified of course.)) Ok, understood. You want me to mess around with malloc just before we try and get XForms 1.1 out of the door. Bad man! I'll have a look. -- Angus
Re: fdesign -convert is buggy
Angus Leeming wrote:
> Lars Gullik Bjønnes wrote:
>> (and I am not saying that -dir should be removed, I am saying that
>> fdesign should output to pwd (unless -dir is specified of course.))
>
> Ok, understood. You want me to mess around with malloc just before
> we try and get XForms 1.1 out of the door. Bad man!
>
> I'll have a look.
This is the current code.
static char const *
filename_only(char const * filename)
{
char const * ptr = strrchr(filename, '/');
if (ptr)
return ptr+1;
return filename;
}
/* Given 'filename' (no extension) and 'ext', generate 'fname' */
static void
build_fname(char * fname, char const * filename, char const * ext)
{
if (fdopt.output_dir) {
strcpy(fname, fdopt.output_dir);
if (fname[strlen(fdopt.output_dir) - 1] != '/')
strcat(fname, "/");
strcat(fname, filename_only(filename));
} else
strcpy(fname, filename);
strcat(fname, ext);
}
I believe that we achieve what you're looking for with
-} else
- strcpy(fname, filename);
+} else {
+ size_t const buffer_size = 512;
+ char buffer[buffer_size];
+ char * cwd = getcwd(buffer, buffer_size);
+ if (cwd) {
+ strcpy(fname, cwd);
+ strcat(fname, "/");
+ strcat(fname, filename_only(filename));
+ } else
+ strcpy(fname, filename);
+}
Test code attached. I'm pretty sure that this is safe, but would value
second opinions...
--
Angus#include
#include
#include
typedef struct {
char * output_dir;
} FD_Opt;
FD_Opt fdopt;
static char const *
filename_only(char const * filename)
{
char const * ptr = strrchr(filename, '/');
if (ptr)
return ptr+1;
return filename;
}
/* Given 'filename' (no extension) and 'ext', generate 'fname' */
static void
build_fname(char * fname, char const * filename, char const * ext)
{
if (fdopt.output_dir) {
strcpy(fname, fdopt.output_dir);
if (fname[strlen(fdopt.output_dir) - 1] != '/')
strcat(fname, "/");
strcat(fname, filename_only(filename));
} else {
size_t const buffer_size = 512;
char buffer[buffer_size];
char * cwd = getcwd(buffer, buffer_size);
if (cwd) {
strcpy(fname, cwd);
strcat(fname, "/");
strcat(fname, filename_only(filename));
} else
strcpy(fname, filename);
}
strcat(fname, ext);
}
int main(int argc, char * argv[])
{
char fname[512];
fdopt.output_dir = 0;
if (argc > 1) {
fdopt.output_dir = (char *)malloc(strlen(argv[1]) + 1);
strcpy(fdopt.output_dir, argv[1]);
}
build_fname(fname, "foo/bar/baz", ".h");
printf("Output fname is \"%s\"\n", fname);
return 0;
}
Re: fdesign -convert is buggy
Angus Leeming <[EMAIL PROTECTED]> writes:
| I believe that we achieve what you're looking for with
| -} else
| - strcpy(fname, filename);
| +} else {
| + size_t const buffer_size = 512;
| + char buffer[buffer_size];
| + char * cwd = getcwd(buffer, buffer_size);
| + if (cwd) {
| + strcpy(fname, cwd);
| + strcat(fname, "/");
| + strcat(fname, filename_only(filename));
| + } else
| + strcpy(fname, filename);
Why isn't filename_only used here?
| static char const *
| filename_only(char const * filename)
| {
| char const * ptr = strrchr(filename, '/');
| if (ptr)
| return ptr+1;
| return filename;
| }
>
| /* Given 'filename' (no extension) and 'ext', generate 'fname' */
| static void
| build_fname(char * fname, char const * filename, char const * ext)
| {
| if (fdopt.output_dir) {
| strcpy(fname, fdopt.output_dir);
| if (fname[strlen(fdopt.output_dir) - 1] != '/')
| strcat(fname, "/");
| strcat(fname, filename_only(filename));
| } else {
| size_t const buffer_size = 512;
| char buffer[buffer_size];
| char * cwd = getcwd(buffer, buffer_size);
| if (cwd) {
| strcpy(fname, cwd);
| strcat(fname, "/");
| strcat(fname, filename_only(filename));
| } else
| strcpy(fname, filename);
| }
| strcat(fname, ext);
| }
It almost seems to me that the else clause should just be
strcpy(fname, filename_only(filename));
and that you don't have to fiddle with getcwd at all?
| int main(int argc, char * argv[])
| {
| char fname[512];
Is this big enough?
(I have no idea what the actual value in xforms is, but PATH_MAX is a
nice value)
| fdopt.output_dir = 0;
| if (argc > 1) {
| fdopt.output_dir = (char *)malloc(strlen(argv[1]) + 1);
| strcpy(fdopt.output_dir, argv[1]);
| }
>
| build_fname(fname, "foo/bar/baz", ".h");
If my above method was used this would produce just baz.h. but the old
way would produce foo/bar/baz.h
--
Lgb
Re: fdesign -convert is buggy
Lars Gullik Bjønnes wrote:
> | } else
> | strcpy(fname, filename);
> | }
> | strcat(fname, ext);
> | }
>
> It almost seems to me that the else clause should just be
>
>strcpy(fname, filename_only(filename));
I think you're correct. Thanks.
> | build_fname(fname, "foo/bar/baz", ".h");
>
> If my above method was used this would produce just baz.h. but the
> old way would produce foo/bar/baz.h
Correct.
The test program seems to work exactly as you want.
Jean-Marc, are you happy with the change?
Index: fdesign/fd_printC.c
===
RCS file: /cvsroot/xforms/xforms/fdesign/fd_printC.c,v
retrieving revision 1.8
diff -u -p -r1.8 fd_printC.c
--- fdesign/fd_printC.c 27 Nov 2003 18:15:00 - 1.8
+++ fdesign/fd_printC.c 27 May 2004 16:50:46 -
@@ -82,7 +82,7 @@ build_fname(char * fname, char const * f
strcat(fname, "/");
strcat(fname, filename_only(filename));
} else
- strcpy(fname, filename);
+ strcpy(fname, filename_only(filename));
strcat(fname, ext);
}
--
Angus#include
#include
#include
typedef struct {
char * output_dir;
} FD_Opt;
FD_Opt fdopt;
static char const *
filename_only(char const * filename)
{
char const * ptr = strrchr(filename, '/');
if (ptr)
return ptr+1;
return filename;
}
/* Given 'filename' (no extension) and 'ext', generate 'fname' */
static void
build_fname(char * fname, char const * filename, char const * ext)
{
if (fdopt.output_dir) {
strcpy(fname, fdopt.output_dir);
if (fname[strlen(fdopt.output_dir) - 1] != '/')
strcat(fname, "/");
strcat(fname, filename_only(filename));
} else
strcpy(fname, filename_only(filename));
strcat(fname, ext);
}
int main(int argc, char * argv[])
{
char fname[PATH_MAX + 1];
fdopt.output_dir = 0;
if (argc > 1) {
fdopt.output_dir = (char *)malloc(strlen(argv[1]) + 1);
strcpy(fdopt.output_dir, argv[1]);
}
build_fname(fname, "foo/bar/baz", ".h");
printf("Output fname is \"%s\"\n", fname);
return 0;
}
