Yes, it's strange to have a boolean function that always return TRUE.
This want_directory function just want to do a check of the path, if
the path is not a directory, then it will print a warning. BTW, I
have a mistake in my patch, the condition in want_directory should be
if (!is_directory(path) && fullwarn)
warning("%s is not a directory", path);
I miss the "!" in my patch.
actually in the original code, because want_directory will always
return true, the following lines in table.c (line 849 - line 886) is
dead code.
if (options[i].syntax == needs_directory
#ifdef KEY
|| options[i].syntax == needs_directory_or_null
#endif
) {
fprintf(f, "\t} else if
(!is_last_char(argv,argi)) {\n");
fprintf(f, "\t\tif (fullwarn) {\n");
fprintf(f, "\t\t\twarning(\"%%s does
not refer to a valid directory\", option_name);\n");
fprintf(f, "\t\t}\n");
fprintf(f, "\t\toptargs =
get_optarg(argv,argi);\n");
fprintf(f, "\t\tget_next_arg(argi);\n");
#ifdef KEY
fprintf(f, "\t\treturn
add_any_string_option(%s,optargs);\n",
options[i].flag);
#else
fprintf(f, "\t\treturn
add_string_option(%s,optargs);\n",
options[i].flag);
#endif
fprintf(f, "\t\t/* NOTREACHED */\n");
}
#ifdef KEY
// Ignore %D? option if no dir arg is found by changing
// them into -dummy.
if (options[i].syntax == needs_directory_or_null) {
fprintf(f, "\t} else {\n");
fprintf(f, "\t optargs =
current_string(argv,argi);\n");
fprintf(f, "\t get_next_arg(argi);\n");
fprintf(f, "\t return O_dummy;\n");
}
#endif
if (options[i].syntax != needs_string
&& options[i].syntax != needs_string_or_dash
) {
fprintf(f, "\t}\n");
}
if (strlen(options[i].name) > 2) {
fprintf(f, "\t}\n");
}
On Thu, Dec 30, 2010 at 10:42 AM, David Coakley <dcoak...@gmail.com> wrote:
> I will not have a chance to look closely until next week, but I have a
> few minor complaints... we can always address these later if the bug
> is blocking progress.
>
> The comment for want_directory() is exactly the same as the comment
> for is_directory(). This looks like an error.
>
> Also, I think it is strange to have a boolean function that always
> returns TRUE. Maybe the code generated by table.c should change
> instead.
>
> -David Coakley / AMD Open Source Compiler Engineering
>
> On Wed, Dec 29, 2010 at 3:18 PM, Sun Chan <sun.c...@gmail.com> wrote:
>> go ahead.
>> Sun
>>
>> On Tue, Dec 28, 2010 at 11:31 PM, Wu Yongchong <wuyongch...@gmail.com> wrote:
>>> Hi, David
>>>
>>> Could you please help review this patch
>>> See https://bugs.open64.net/show_bug.cgi?id=709
>>>
>>> In revision 3437 , the function want_directory has been remove
>>> considering it's a dead function, but it's not true. This function
>>> will always return TRUE no matter the path is a directory or not.
>>> If it's replace with is_directory, the following situation will fail .
>>>
>>> There is no directory named libs
>>> $ opencc -I ./libs test.c
>>> opencc WARNING: unknown flag: -I
>>> gcc: ./libs: No such file or directory
>>>
>>>
>>> Here is the patch.
>>> Index: osprey/driver/file_utils.c
>>> ===================================================================
>>> --- osprey/driver/file_utils.c (revision 3437)
>>> +++ osprey/driver/file_utils.c (working copy)
>>> @@ -163,6 +163,13 @@
>>> return FALSE;
>>> }
>>>
>>> +boolean want_directory (char *path)
>>> +{
>>> + if (is_directory(path) && fullwarn)
>>> + warning("%s is not a directory", path);
>>> + return TRUE;
>>> +}
>>> +
>>> /* check if directory is writable */
>>> boolean
>>> directory_is_writable (char *path)
>>> Index: osprey/driver/file_utils.h
>>> ===================================================================
>>> --- osprey/driver/file_utils.h (revision 3437)
>>> +++ osprey/driver/file_utils.h (working copy)
>>> @@ -61,6 +61,9 @@
>>> /* check whether is a directory */
>>> extern boolean is_directory (char *path);
>>>
>>> +/* check whether is a directory */
>>> +extern boolean want_directory (char *path);
>>> +
>>> /* check whether directory is writable */
>>> extern boolean directory_is_writable (char *path);
>>>
>>> Index: osprey/driver/table.c
>>> ===================================================================
>>> --- osprey/driver/table.c (revision 3437)
>>> +++ osprey/driver/table.c (working copy)
>>> @@ -827,7 +827,7 @@
>>> || options[i].syntax ==
>>> needs_directory_or_null
>>> #endif
>>> ) {
>>> - fprintf(f, "\tif
>>> (is_directory(next_string(argv,argi))) {\n");
>>> + fprintf(f, "\tif
>>> (want_directory(next_string(argv,argi))) {\n");
>>> }
>>> fprintf(f, "\t\toptargs = get_optarg(argv,
>>> argi);\n");
>>> if (options[i].syntax == needs_decimal) {
>>>
>>>
>>> --
>>> yongchong
>>>
>>> ------------------------------------------------------------------------------
>>> Learn how Oracle Real Application Clusters (RAC) One Node allows customers
>>> to consolidate database storage, standardize their database environment,
>>> and,
>>> should the need arise, upgrade to a full multi-node Oracle RAC database
>>> without downtime or disruption
>>> http://p.sf.net/sfu/oracle-sfdevnl
>>> _______________________________________________
>>> Open64-devel mailing list
>>> Open64-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/open64-devel
>>>
>>
>
--
yongchong
------------------------------------------------------------------------------
Learn how Oracle Real Application Clusters (RAC) One Node allows customers
to consolidate database storage, standardize their database environment, and,
should the need arise, upgrade to a full multi-node Oracle RAC database
without downtime or disruption
http://p.sf.net/sfu/oracle-sfdevnl
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
