Re: programs:extrac32 Add new program (Extrac32.exe) (Pre alpha version :))

[Vitaliy Margolen]

Anatoly Lyutin wrote:
> This program (EXTRAC32.exe) needed for extract *.cab files. Some Win 
> programs use EXTARC32.exe for inner functions and often impossible to 
> change unpacker. I test this EXTRAC32.exe for one program (Russian 
> office program) and it works!
>
> I shall be glad for all comments which will help to make this program 
> better.
> +static BOOL ExtCmp( const WCHAR str1[], const WCHAR str2[] )

Any reason you are not using strcmpW?
> +LRESULT WINAPI ExtCabCallback ( IN PVOID pMyInstallData,
> -
>                 IN UINT Notification,
> -
>                 IN UINT Param1,
> -
>                 IN UINT Param2 )
>
> +{

What do you need the "IN" here for? It's the useless domain of some parts of 
PSDK.

Please use one format of indentation, if (), function( p1, p2, p3 ), etc. 
It's hard to impossible to read your code with at least 5-10 different 
variations of both some times IN ONE LINE!
> -
>     case SPFILENOTIFY_NEEDNEWCABINET: // Unexpected.
> -
>        lRetVal = NO_ERROR;
> -
>        break;
> -
>  }

Do not use c++ comment stile. This part is completely useless - you set 
lRetVal = NO_ERROR; above already. Same type of mistake all over the place.
> +++ programs/extrac32/parsecmd.c	2007-07-04 16:12:38 +0400

What do you need a whole separate file for? It has only one function!
> - for(i=0;i<strlenW(cmdline);i++)
> - {
> -
>      if( ((cmdline[i] == ' ')||(cmdline[i] == '\t') )&&(!bCav)&&((i!=0)&&(cmdline[i-1]!=' ')&&(cmdline[i-1]!='\t')) )
> -
>      {

This will never be true with i == 0, so you might as well start from i = 1.
> - if( (cmdline[strlenW(cmdline)-1] != ' ')&&(cmdline[strlenW(cmdline)-1] != '\t') )
> -
>    (*argc)++;

What if cmdline is ""? What will be be checking here? Also you really should 
combine this with the previous for() loop.
> -
>    if(cmdline[i] == '\"')

You don't need to escape double-quote here.

Vitaliy.

programs:extrac32 Add new program (Extrac32.exe) (Pre alpha version :))

[Anatoly Lyutin]

Vitaliy Margolen wrote:
> Anatoly Lyutin wrote:
> > This program (EXTRAC32.exe) needed for extract *.cab files. Some Win 
> > programs use EXTARC32.exe for inner functions and often impossible to 
> > change unpacker. I test this EXTRAC32.exe for one program (Russian 
> > office program) and it works!
> >
> > I shall be glad for all comments which will help to make this program 
> > better.
> > +static BOOL ExtCmp( const WCHAR str1[], const WCHAR str2[] )
>
> Any reason you are not using strcmpW?

Yes, it is. In ExtCmp() I use CompareString with flag NORM_IGNORECASE. 
It is more conveniently to set option in various case.
> > +LRESULT WINAPI ExtCabCallback ( IN PVOID pMyInstallData,
> > -
> >                 IN UINT Notification,
> > -
> >                 IN UINT Param1,
> > -
> >                 IN UINT Param2 )
> >
> > +{
>
> What do you need the "IN" here for? It's the useless domain of some 
> parts of PSDK.
>
> Please use one format of indentation, if (), function( p1, p2, p3 ), 
> etc. It's hard to impossible to read your code with at least 5-10 
> different variations of both some times IN ONE LINE!

I will correct it.
> > -
> >     case SPFILENOTIFY_NEEDNEWCABINET: // Unexpected.
> > -
> >        lRetVal = NO_ERROR;
> > -
> >        break;
> > -
> >  }
>
> Do not use c++ comment stile. This part is completely useless - you 
> set lRetVal = NO_ERROR; above already. Same type of mistake all over 
> the place.

Ok.
> > +++ programs/extrac32/parsecmd.c    2007-07-04 16:12:38 +0400
>
> What do you need a whole separate file for? It has only one function!

Somebody knows function to parse cmd line with quote? It is not a good 
idea to write similar functions for different programs...
And I separate this function into other file because this function may 
be use in another program. I think that it is not correct to join 
program with this function because it is more conveniently for testing 
this function and etc.
> > - for(i=0;i<strlenW(cmdline);i++)
> > - {
> > -
> >      if( ((cmdline[i] == ' ')||(cmdline[i] == '\t') 
> >
> > )&&(!bCav)&&((i!=0)&&(cmdline[i-1]!=' ')&&(cmdline[i-1]!='\t')) )
> > -
> >      {
>
> This will never be true with i == 0, so you might as well start from i 
> = 1.

Hmm. I missed it.
> > - if( (cmdline[strlenW(cmdline)-1] != '
> >
> > ')&&(cmdline[strlenW(cmdline)-1] != '\t') )
> > -
> >    (*argc)++;
>
> What if cmdline is ""? What will be be checking here? Also you really 
> should combine this with the previous for() loop.

It will be corrected.
> > -
> >    if(cmdline[i] == '\"')
>
> You don't need to escape double-quote here.

I do not see distinctions in representation of this symbol.  Escaped 
double-quote better for view  in code than  '"'. IMHO
> Vitaliy.

Thank you for your answer.