FWD » Core Development » Base Language

After testing the simple case of OS-DIR usage (no options)

define stream s.
def var ch as char.
input stream s from OS-Dir ("c:\my-dir") echo.
repeat:
  import stream s unformatted ch.
  message ch.
end.
output stream s close.

OS-DIR seems to output the list of directory and files paths contained inside the specified directory, in this format:

 "Quoted_File_Name" "Quoted_File_Path" F
 "Quoted_Dir_Name" "Quoted_Dir_Path"  D
 "Quoted_Device_Name" "Quoted_Special_Device" S
 "Quoted_Unknown_Type_Name"  "Quoted_unknown_type_path" X
 "Quoted_Hidden_File_Name" "Hidden_File_path" H
 "Simbolic_link_name" "Simbolic_Link_Path" L
 "Pipe_File_Name" "Pipe_File_Path" P
 "Member_of_Prolib" "Member_of_Prolib" 

Some of the index are multiplicative for example a hidden file will have this output
"Quoted_Hidden_File_Name" "Hidden_File_path" FH
but a hidden directory will have:
"Quoted_Hidden_Dir_Name" "Quoted_Hidden_Dir_Path" D

Some of the results are Unix specific(pipe file and also special device seems to be in this category).

• NO-ATTR-LIST
  This Will give the result without the type identification index (only entity name and path) )

• BINARY
  Input is read directly without conversion or interpretation.(Still to determine if there a case where there is any difference in output with this mode and normal.

• ECHO
  This should display input data on the current output device (on by default). (Still to determine a case where this happens -no data was output in my tests and there doesn't seem to be other ways of calling OS-DIR - tried OS-Command)
• NO-ECHO
  This does the work without outputting to output device. It doesn't change behavior from ECHO.

• MAP protermcap-entry
  protermcap-entry value is an entry from the PROTEMRCAP file.
  This is to be used for reading from an input stream that uses different character translation from the current one.
  (Test case to be determined if necessary.)

• UNBUFFERED
  Only usable when mixing input from a Unix process (Test case to be determined if necessary)

• CONVERT
  Allows to modify the character conversion. (Test case to be determined if necessary)

• TARGET target-codepage
  Specifies the character conversion code page of the target (Test case to be determined if necessary)

• SOURCE target-codepage
  Specifies the character conversion code page of the source(Test case to be determined if necessary)

• NO-CONVERT
  No character conversion (Test case to be determined if necessary)

Updated the Input from OS-Dir Specifications.
Certain things are not 100% clear:
From the list of arguments only NO-ATTR-LIST seems to be logically related to OS-DIR, are the rest of arguments related to INPUT FROM?
If so, can they be ignored as out of scope of this task?
If other arguments form the list besides NO-ATTR-LIST are to be handled by OS-DIR logic like map , convert, target and source , more research would be required.

The solution I propose for this statement INPUT STREAM s FROM OS-DIR(location) "ARGS" is be to add and call something like this
sStream.assign(StreamFactory.getStreamFromOSDir("path" ,..., arglist[]). Where the arglist may contain NO-ATTR-LIST or also the other attributes if required.

From the list of arguments only NO-ATTR-LIST seems to be logically related to OS-DIR, are the rest of arguments related to INPUT FROM?

Yes.

If so, can they be ignored as out of scope of this task?

Yes. But you should understand WHY this is the case. Please write a couple simple testcases where UNBUFFERED or BINARY is used. Convert them and look at the result.

Then search for prog.io_options in rules/convert/input_output.rules to see how we implement our options.

The solution I propose for this statement INPUT STREAM s FROM OS-DIR(location) "ARGS" is be to add and call something like this
sStream.assign(StreamFactory.getStreamFromOSDir("path" ,..., arglist[]). Where the arglist may contain NO-ATTR-LIST or also the other attributes if required.

We generally don't put options into the StreamFactory method calls. However, in this case I think it is OK for NO-ATTR-LIST to be implemented as a boolean literal.

As far as I can tell, there should never be a need for more than 2 parameters. One parameter is required (the path). Is that correct?

If so, then please make these methods:

StreamFactory.openOSDir(String) this is the version that gets called if there is no NO-ATTR-LIST option.
StreamFactory.openOSDir(character) this is an alternate of the first one
StreamFactory.openOSDir(String, boolean) this gets called when there is a NO-ATTR-LIST option
StreamFactory.openOSDir(character, boolean) this is an alternate of the 3rd one

From there, you must implement a new stream subclass that is returned by the factory.

From default conversion logic p2j will also add 2 boolean parameters when calling StreamFactory.openOsDir: boolean write, boolean append which don't seem relevant to the OS-Dir context. Can they be ignored in the case of OS-Dir.

Those parameters should NOT be emitted in the OS-DIR case.

Java (at least until Java 7 NIO 2) doesn't seem to have a way of detecting symbolic links( the problem it's likely the same with devices and nonFIFO pipes).
From research a solution could be to use JNI to call code from a shared object which will be built with the other native code for the target OS.
Is this solution ok?

Yes, this solution is OK. We already have a libp2j.so (which we will have to also port to Windows as p2j.dll). Right now, there is only the 1 src/native/process.c object there. You would add a new src/native/filesys.c with the proper JNI functions (and the src/native/makefile will also need updates). Please keep that interface as simple as possible and document the signature(s) here before implementing. Also note that JNI calls are costly, so the number of calls should be kept to a minimum where it can be done without making everything more complicated.

This also requires some changes to the ant task "native" ,
otherwise I made the changes and tested for symbolic links and it seems to work fine, will research to add for pipes and devices also.

Good. Please post your prototype code for review. Just paste the code into your redmine issue update if it is small enough to easily fit.

The code for filesys.c

/*
** Module   : filesys.c
** Abstract : implementation for detection of simbolic links, non-FIFO pipes
**            and special devices.
**            This is used by com.goldencode.p2j.utilFileChecker class
**
** Copyright (c) 2013, Golden Code Development Corporation.
** ALL RIGHTS RESERVED. Use is subject to license terms.
**
**           Golden Code Development Corporation
**                      CONFIDENTIAL
**
** -#- -I- --Date-- ---------------------Description----------------------
** 001 CS  20130104 Initial revision of the file. 
*/ 
    #include <stdio.h>
    #include <sys/stat.h>
    #include <unistd.h>
    #include <sys/types.h>
    #include <jni.h>
    #include "Filesys.h" 

    /*
     * Utility method to check if the given file path is a symbolic link.

     * @param  env 
     *         pointer to a structure that provides access to JNI environment
     * @param  jc 
               reference to the Java class who called this function
     *         (from static context)

     * @param  fname 
     *         The file name to be checked passed from FileChecker.isLink().

     * @return a wrapped integer 1 for true , false otherwise
     */
JNIEXPORT jint JNICALL 
       Java_com_goldencode_p2j_util_FileChecker_isLink (JNIEnv  *env,
                                                        jclass  jc, 
                                                        jstring fname) 
{
   const char *c_string = (*env)->GetStringUTFChars(env, fname, NULL);
   if (c_string == NULL) 
   {
      return -2; /* OutOfMemoryError already thrown */
   }

   struct stat buf;

   //get symbolic link status
   int lnk_stat = lstat(c_string,&buf); 

   (*env)->ReleaseStringUTFChars(env, fname, c_string);
   //if link status is -1 return -1
   if (lnk_stat==-1) return -1;

   //call s_islnk function which determine if symbolik link
   return S_ISLNK(buf.st_mode);
}

Actually , I realize this will have to be tested on a windows system since there might be problems with lstat().

It is OK to be linux-specific right now. We will have separate tasks to port this to Windows.

Some initial feedback:

1. Please clean up the coding standards deviations. It is close, but not quite right.

2. What is in Filesys.h?

3. Where is the #include "com_goldencode_p2j_util_FileChecker.h" that is the generated include file created by the JDK for the native methods?

4. On the basic design, I can accept a simple 1-file per call approach for now. BUT the same function should return a value that reports on the query of all attributes (symlink, device, pipe). In addition, I want a return value that is platform independent so that the on Windows the function would return the same data. That way the Java code can be independent of the platform, with the platform differences hidden in the JNI C code. Please use a bitfield approach. First, add symbolic definitions for the bits:

#define SYMLINK 1
#define DEVICE 2
#define PIPE 4

Then the return value can be OR'd together to create an integer whose bits can be checked as a flag.

That reduces the number of "trips" through JNI and makes the return value platform independent in a single approach.

For Windows stat() seems to do the job, but in Unix lstat() must be used or link will not be corectly detected.
2-> 3 in the ant native task I used javah to output the header to Filesys.h instead of com_goldencode_p2j_util_FileChecker.h but i can change it to that.
4. I'll change the method to this approach

The pipes case may be a problem to detect, since files and FIFO pipes are both marked with "F" only nonFIFO pipes will appear as "P according to Progress specification.

The problem is all the API's I've found regarding pipes detection only refer to FIFO pipes.
Non FIFO pipes are specific only to Unix, so I couldn't test how Progress will handles it.

Even more in Windows FIFO pipes cannot be created by command as files in a writeable filesystem so I cannot test in this case either if they are in fact recognized as files and not pipes as the documentation specifies.

Devices are also Unix specific but in this case there are more specific API for detection.

Member of Prolib "P" type is related to the Progress's library so it's not useful in any way to implement this.

What would be the best solution for the cases that can't be tested against Progress windows:
1. Do not treat them - mark as unknown "X" everything that does not classify as a file,hidden, directory or symbolic link.
2. Create implementation for pipes and devices even if they can't be tested against Progress behavior.

Implement option 2 (create the Linux implementation even though we cannot test the 4GL on Linux today). Please note that we will soon have access to a 4GL Linux environment and you will be able to fully test this. But for now, do the implementation and make it work the way it is documented to work. We will fix it later if the documentation is wrong.

Don't worry about the Progress library type. You are right that we won't support that. Devices should be supported.

In regards to the P "Pipe file" attribute, I also wondered what the Progress documentation means. A FIFO in Linux/UNIX is a "named pipe" and it is listed in the file system just like a file. I googled around and found some discussion that suggests that you might see BOTH F and P attributes for a FIFO, but you will only see an F (with no P) for regular files. Please implement with this as the assumption.

As far as I understand it, you cannot see a non-FIFO pipe in the filesystem. So there is never a case where something created using the UNIX/Linux pipe() function will be returned from OS-DIR, so you don't have to worry about this case.

To be clear, I think that when Progress says "You might also get one or more of the following characters:" they mean that the L, H and P attributes are always IN ADDITION TO the main attribute F, D, S or X. The latter are listed as "You will get one of the following characters:", so I assume you will always have ONE of the FDSX and that you MAY ALSO have one (or more?) of the LHP.

... they mean that the L, H and P attributes are always IN ADDITION TO the main attribute F, D, S or X

Yes those attributes should not appear alone however there is not a consistent behavior in which they appear.
For example a hidden file will get FH but a hidden directory will get only D.

On windows Progress symbolic links are not even marked , the results of test are:

symbolic-link-to-dir  -> D
symbolic-link-to-file -> F

If it worked according to specification the following results would be expected

symbolic-link-to-dir  -> DL
symbolic-link-to-file -> FL

It is likely there is a different behavior from Unix to windows when detecting file types otherwise why would spec say it does mark symbolic links?

I will implement according to spec at the moment and modify when we can test if it doesn't match.

The C implementation "filesys.c" according to Progress documentation

/*
** Module   : filesys.c
** Abstract : implementation for detection of simbolic links, non-FIFO pipes
**            and special devices.
**            This is used by com.goldencode.p2j.utilFileChecker class
**
** Copyright (c) 2013, Golden Code Development Corporation.
** ALL RIGHTS RESERVED. Use is subject to license terms.
**
**           Golden Code Development Corporation
**                      CONFIDENTIAL
**
** -#- -I- --Date-- ---------------------Description----------------------
** 001 CS  20130104 Initial revision of the file. 
*/ 
#include <stdio.h>
#include <sys/stat.h>
#include <unistd.h>
#include <sys/types.h>
#include <errno.h> 
#include <string.h> 
#include <fcntl.h> 
#include <jni.h>
#include "com_goldencode_p2j_util_FileChecker.h" 

/**
* Utility method to check if the given file path is a symbolic link.

* @param  env 
*         pointer to a structure that provides access to JNI environment
* @param  jc 
*         reference to the Java class who called this function
*         (from static context)
*
* @param  fname 
*         The file name to be checked passed from FileChecker.isLink().
*
* @return a wrapped integer 1 for true , false otherwise
*/
JNIEXPORT jint JNICALL 
       Java_com_goldencode_p2j_util_FileChecker_checkFileType(JNIEnv  *env,
                                                        jclass  jc, 
                                                        jstring fname) 
{

   const char *c_string = (*env)->GetStringUTFChars(env, fname, NULL);
   if (c_string == NULL) 
   {
      return -2; /* OutOfMemoryError already thrown */
   }

   struct stat buf;

   int status= -1;

   //get symbolic link status
   int lnk_stat = lstat(c_string,&buf); 

   (*env)->ReleaseStringUTFChars(env, fname, c_string);
   //if link status is -1 return -1
   if (lnk_stat==-1) return -1;

   // check for file
   if (S_ISREG(buf.st_mode))
   {
      // Status for file
      status = 1;
   }

   // check for dir
   if (S_ISDIR(buf.st_mode))
   { 
      // Status code for directory
      status =  2;
   }

   // check for special devices
   if (S_ISCHR(buf.st_mode) || S_ISBLK(buf.st_mode) )
   { 
      // status code for devices
      status = 3;
   }
   // check for symbolic links
   // NOTE: A symbolic link that points to a directory or file will not be
   // detected as file or directory by the logic present here so this status 
   //cannot be made inclusive with file or directory
   if( S_ISLNK(buf.st_mode))
   {
      //status code for symbolic link
      status = 4;
   }

   // check for fifo pipe
   if (S_ISFIFO(buf.st_mode))
   {
      // Pipe code status
      status = 5;
   }

   //Check if file doesn't match either of the possible types
   if (!S_ISFIFO(buf.st_mode) && !S_ISSOCK(buf.st_mode) && 
       !S_ISLNK(buf.st_mode) && !S_ISCHR(buf.st_mode) && 
       !S_ISREG(buf.st_mode) && !S_ISDIR(buf.st_mode))
   {
      // unknown code status
      status = 0;
   }
   return status; 
}

Originally I wanted to give an array containing the file names and return the array of types(to minimize the number of calls),
However symbolic links that point to files and directories are not also detected as directories so creating a composed type like DL
couldn't be possible.
The Pipe type : P is considered as named FIFO pipe , character and block devices as Special device type: S, and anything that doesn't match as unknown type : "X" until this can be confirmed by test.

This is a step in the right direction, but it is incomplete or wrong in some ways.

1. I don't like that you have made the status codes mutually exclusive instead of using a bitfield. Even if you can make it work in this case, it seems to me that it is not logically correct and thus it is confusing to future readers.

2. I don't see how you can ever get the result FL or DL using your code. Perhaps you are relying upon other code in J2SE to get to that result? I prefer to have the entire attributes calculation done in this one function. This will avoid the need to call other J2SE methods and merge the results in Java. This would make this code easier to understand because it will map better to the requirements of Progress rather than splitting the logic into multiple places. It also may have a performance advantage, since J2SE will have to call down through JNI to get the same information anyway. So by doing it all at once, we should get an improvement. Of course, you will have to do something more than just the lstat() to do the job, but that should not be hard.

3. I don't like your approach with the "unknown type":

   //Check if file doesn't match either of the possible types
   if (!S_ISFIFO(buf.st_mode) && !S_ISSOCK(buf.st_mode) && 
       !S_ISLNK(buf.st_mode) && !S_ISCHR(buf.st_mode) && 
       !S_ISREG(buf.st_mode) && !S_ISDIR(buf.st_mode))
   {
      // unknown code status
      status = 0;
   }

There are multiple problems here. 1st, S_ISSOCK() doesn't correspond to any other return code, so if you have a socket then it will NOT return 0. That seems wrong. 2nd, we don't want or need to replicate the testing code here. Instead, just initialize status to 0 at the top of the function. Then remove this unnecessary testing code. That way, if none of the other tests turn on bits, then the result is 0 and you can translate that into UNKNOWN naturally.

4. The return code documentation is wrong. When you fix it, you should fully detail how to interpret the return code. Please reference the symbolic constants (and also their numeric values in parenthesis) for each bitfield.

1. I don't like that you have made the status codes mutually exclusive..
2. I don't see how you can ever get the result FL or DL using your code. Perhaps you are relying upon other code in J2SE to get to that result?

Yes I rely on code from java because detection in this case is exclusive -> a symbolic link pointing to a directory will not check as directory with S_ISDIR.

Initially I wanted to do the same thing (have the entire type logic in filesys.c) and when this approach didn't work because of type exclusiveness I've tried using fstat() function and file properties unfortunately that approach didn't correctly detect symbolic links among some other things.

The draft solution showed here uses checkFileType function only for secondary types: symbolic links , and pipes as well as main types like special device and unknown which can't classify as the previous secondary types.

The code for handling this in Java is

  ..
  String type = "";
      if (file.isFile())
      {
         type = "F";
         // H is additive type index but only applies to file
         if (file.isHidden())
         {
            type = type + "H";
         }
      }
      else if (file.isDirectory())
      {
         type = "D";
         // Hidden directories don't seem to be matched as DH
      }
      // the other type index are additive or not supported by java

      type = type + FileChecker.getFileType(file.getAbsolutePath());
      ...

where FileChecker.getFileType(file.getAbsolutePath()) will return the type letters: L P S X ,or and empty string according to defined types.

Getting the whole file type logic inside filesys.c might be achievable by combining multiple methods of detection to get non-excluding results(I thought this might be more complex and confusing than the mixed java solution) I can start working towards that.

3.Sounds right to do it this way, initially I put it -1 to mark error in case something goes wrong but it isn't the case.

4. Will change to the correct documentation

Other than that tested it and seems to function as resulting from documentation and available Progress tests.

Finished and tested the filesys.c version that handles all the logic inside the c code using bitfield approach for status (will add it once I finish cleanup the code).
What I wanted to ask is how it will be handled for multiple OS, I was thinking we can have 2 versions filesys_unix.c and filesys_win.c and makefile will decide which one to take depending on OS - some functions I used don't exist for win and it might not compile if we try to handle in the same file logic for both OS.

We will handle that in a different task. We will likely use platform specific files with functions that are called from filesys.c and the correct version will be compiled in by the makefile, as you say.

Please upload the proposed update as soon as you have it.

Feedback:

1. In input_output.rules, this change seems wrong:

      <!-- create the 3rd parameter for FileStream construction -->
      <rule> 
         ((type == prog.kw_to and parent.type == prog.output_to) or
          (type == prog.kw_from and parent.type == prog.input_from)) and
         !descendant(prog.kw_term, 1) and this.firstChild.type != prog.kw_os_dir
        <rule>appendmode
            <action on="false">
               createJavaAst(java.string, text, closestPeerId)
            </action>

I think this will break the standard case of opening a file in overwrite mode. The 3rd parameter is supposed to be a boolean for that case. You have changed it to a string. What is the idea here?

2. Some formatting problems in FileChecker and OsDirStream:

 ** 001 CS  20130103 Created initial version for Implement OS-DIR support as
 *                   an INPUT stream task.
 */
package com.goldencode.p2j.util;

/**
 * <p>
 * Utility class that uses JNI call to get the bitfield representing the 

Use ** instead of * in front of each line in the header.
Put an extra blank line between the header and the package stmt.
Don't start the javadoc with a <p>.

The code should look like this:

 ** 001 CS  20130103 Created initial version for Implement OS-DIR support as
 **                  an INPUT stream task.
 */

package com.goldencode.p2j.util;

/**
 * Utility class that uses JNI call to get the bitfield representing the 

The 1st two of these problems also exist in OsDirStream.

3. In FileChecker SECUNDARY is spelled SECONDARY. This spelling error is in OsDirStream too.

4. The FileChecker.getFileType() is OK, but not the preferred way to implement. The approach you have coded can only match a subset of the bitfields that could be returned. If you have found all possible combinations, then that is OK, but it certainly makes the code a bit more complicated that needed. Only the "primary" types are mutually exclusive. But the way you have coded it, everything is mutually exclusive. This makes the code more fragile than needed.

Normally, one would just decode the bitfield, bit by bit:

public class FileChecker
{
   /** Status code for error */
   private static final int ERROR_TYPE =-1;

   /** Status bitfield code for unknown */
   private static final int UNKNOWN_TYPE = 0;

   /** Status bitfield code for directory */
   private static final int DIRECTORY_TYPE = 1<<0;    // 000001

   /** Status bitfield code for file */
   private static final int FILE_TYPE = 1<<1;         // 000010

   /** Status bitfield code for symbolic link */
   private static final int SYMBOLIC_LINK_TYPE = 1<<2; //000100

   /** Status bitfield code for devices */
   private static final int DEVICE_TYPE = 1<<3;       // 001000

   /** Status bitfield code for pipes */
   private static final int PIPE_TYPE = 1<<4;         // 010000

   /** Status bitfield code for hidden */
   private static final int HIDDEN_TYPE = 1<<5;       // 100000

   // load the native library
   static
   {
      System.loadLibrary("p2j");
   }

   /**
    * Native utility method that will return a bitfield code representing the
    * status of the file.
    * 
    * @param   filePath
    *          The String representing the path of the file for
    *          which we want to calculate the status code.
    * 
    * @return  The status bitfield code of the file.
    */
   public native static int checkFileType(String filePath);

   /**
    * This method will call the native code handling the file type detection,
    * interpret the bitfield result and return the status in Progress format.
    * 
    * @param  filePath
    *         The String representing the path of the file for
    *         which we want to calculate the status code.
    *         
    * @return The composed status of the file.
    */
   public static String getFileType(String filePath)
   {
      StringBuilder sb = new StringBuilder();

      int val = checkFileType(filePath);

      if (val == -1)
      {
         // we probably should raise an ErrorCondition here
      }

      // TODO - Re-check Progress Unix behavior for file type, when available.

      // primary type checking      
      if (val & FILE_TYPE)
      {
         sb.append('F');
      }
      else if (val & DIRECTORY_TYPE)
      {
         sb.append('D');
      }
      else if (val & DEVICE_TYPE)
      {
         sb.append('S');
      }
      else
      {
         // the special case of UNKNOWN_TYPE
         return "X";
      }      

      // secondary type checking (the order is important here)
      if (val & HIDDEN_TYPE)
      {
         sb.append('H');
      }
      if (val & SYMBOLIC_LINK_TYPE)
      {
         sb.append('L');
      }
      if (val & PIPE_TYPE)
      {
         sb.append('P');
      }

      return sb.toString();      
   }
}

Please note how the code is shorter and less fragile.

5. In FileChecker.getFileType(), what is the case where Progress will only return an "L"? Your current code allows this, but as far as I understand it, it should not be possible.

6. In FileChecker.getFileType(), there needs to be code to handle the -1 (error case). I stubbed it out in my example, but we need to do something there.

7. Please change the names of the classes and methods to from OsDirStream to DirStream and from openOsDirStream to openDirStream().

8. In OsDirStream, there are some formatting issues:

This:

public class OsDirStream
extends
Stream
{

should be this:

public class OsDirStream
extends Stream
{

This:

    * </ul>
    * <p>
    * @param dir

should be this:

    * </ul>
    *
    * @param dir

This:

   public long available()
                          throws IOException
   {

should be this:

   public long available()
   throws IOException
   {

This:

   public long getPos()
                       throws UnsupportedOperationException,
                          IOException
   {

should be this:

   public long getPos()
   throws UnsupportedOperationException,
          IOException
   {

This:

   {
      throw new UnsupportedOperationException(INPUT_READER_ONLY);

   }

should be this:

   {
      throw new UnsupportedOperationException(INPUT_READER_ONLY);
   }

This:

      catch (IOException e)
      {

      }

   }

should be:

      catch (IOException e)
      {
         // best efforts, so ignore errors
      }
   }

This:

   private void addOsDirLine (File file , StringBuffer sb, boolean noAttrList)
   {

should be (there is a space removed in 2 places):

   private void addOsDirLine(File file, StringBuffer sb, boolean noAttrList)
   {

Many of these problems appear in multiple places so please fix the issues everywhere.

9. In OsDirStream, I don't think the use of UnsupportedOperationException will give the proper error handling. In Progress, what happens if you try to write to a stream that is defined using INPUT FROM OS-DIR? What happens when you try to get or set the seek position? I am pretty sure that the 4GL will give you a very specific error message. We should duplicate this (probably using the recordOrThrowError()).

10. In the addOsDirLine(), you are "unwrapping" the character var's String using getValue(). That is not a good practice. Please use toStringMessage() for this.

   private void addOsDirLine (File file , StringBuffer sb, boolean noAttrList)
   {
      sb.append(character.quoter(file.getName()).getValue() + " " + 
      character.quoter(file.getAbsolutePath()).getValue() + " " + 
      (noAttrList ? "" : FileChecker.getFileType(file.getAbsolutePath())));
   }

11. Have you tested to make sure that the last entry in a directory list is properly handled? There is no final NEWLINE added at the end of the stream and I wonder if that is different from the 4GL.

12. In code like this:

         int errNo = 293;
         ErrorManager.recordOrThrowError(errNo, errMsg);

there is no advantage to the errNo variable here. Just hard code it:

         ErrorManager.recordOrThrowError(293, errMsg);

13. In StreamBuilder, please remove the T column from the header.

14. In StreamDaemon, there is this code:

   public int openOSDirStream(String dir, boolean noAttrList)
   {

      return store(new OsDirStream(dir, noAttrList));
   }

it should be this:

   public int openOSDirStream(String dir, boolean noAttrList)
   {
      return store(new OsDirStream(dir, noAttrList));
   }

This same formatting problem exists in multiple places in the StreamFactory file.

15. What is this code that has been added to StreamFactory?

   /**
    * Writes data to a stream
    *  
    * @param s
    *        The stream to write to
    * @param data
    *        The data to be written
    */
   private static void writeToStream(Stream s, String data)
   {
      s.export(new FieldEntry[]
      {
             new ExportField(data)
      });
   }

16. In filesys.c, where you use a feature like this:

status = status | SYMLINK;

It is cleaner to write it like this:

status |= SYMLINK;

17. In filesys.c, #define of a constant should be outside of the function.

18. In filesys.c, there are many formatting problems:

- The indents are all 4 chars but they should be 3 chars.
- There are many places where there is missing space like if() instead of if () and also like lstat(file_path,&buf) instead of lstat(file_path, &buf).
- There are places where there is extra space like if( S_ISLNK(buf.st_mode)) instead of if (S_ISLNK(buf.st_mode))
- There are some extra blank lines where there should be none.
- This:

JNIEXPORT jint JNICALL 
       Java_com_goldencode_p2j_util_FileChecker_checkFileType(JNIEnv  *env,
                                                        jclass  jc, 
                                                        jstring fname) 
{

should be this:

JNIEXPORT jint JNICALL 
   Java_com_goldencode_p2j_util_FileChecker_checkFileType(JNIEnv  *env,
                                                          jclass  jc, 
                                                          jstring fname) 
{

6. In FileChecker.getFileType(), there needs to be code to handle the -1 (error case). I stubbed it out in my example, but we need to do something there.

The only scenario in which the -1 error case can happen would be if something happen to one of the file/directory/device in the OS-DIR after it was listed but before it was processed. Unfortunately I haven't been able to reproduce this in Progress to see how this is handled.
We could use ErrorManager to throw an error - we don't know the equivalent Progress , or we could return an empty status.

Otherwise I've fixed the other points and clean-up the code from parts I used for test and didn't make sense in the context and fixed what seemed to be a bug in StreamWrapper which resulted in 2 confusing error messages when trying to write to DirStream.

Commited testcases to bzr.

Unfortunately I haven't been able to reproduce this in Progress to see how this is handled.
We could use ErrorManager to throw an error - we don't know the equivalent Progress , or we could return an empty status.

Use ErrorManager.recordOrThrowError() and just use an error # of -1 and some reasonable text to describe the problem. Put a TODO comment in there to explain that we don't know how 4GL handles this because we haven't been able to duplicate the condition.

Upload the code when you have it ready.

I have 2 questions:

1. Where is the StreamWrapper change? The one that is included in the update is identical with the one already checked into bzr.

2. What is the status of your unit testing of this update?

Otherwise the changes look good.

1. I checked the StreamWrapper from proposed update against bzr and the changes are on method assignStream(Stream s)
lines 394-395:

new

inOk  = stream.isIn();
outOk = stream.isOut();

inOk  = true;
outOk = true;

The results of the tests are ok, ran the conversion tests and there weren't any differences.

You are right. I must have been comparing the wrong StreamWrapper file.

The results of the tests are ok, ran the conversion tests and there weren't any differences.

Let's change this code a bit:

      sb.append(character.quoter(file.getName()).toStringMessage() + " " + 
      character.quoter(getPathName(file)).toStringMessage() + " " + 
      (noAttrList ? "" : FileChecker.getFileType(file.getAbsolutePath())));

change it to this:

      sb.append(character.quoter(file.getName()).toStringMessage())
        .append(' ')
        .append(character.quoter(getPathName(file)).toStringMessage());

      if (!noAttrList)
      {
         sb.append(' ')
           .append(FileChecker.getFileType(file.getAbsolutePath()));
      }

It is longer, but much easier to read this way. Also: we don't add the extra space at the end when noAttrList is true. I suspect that Progress would not add the extra space in that case. Unless your testing has shown otherwise, we would normally do it this way.

Everything else looks good, including your latest change. Please prepare the final update and re-test with the OS-DIR testcases.

If that passes, update the history here and then get the conversion regression testing done. If that passes, then you will need to get this regression tested using the harness (the automated runtime regression testing). Please work with Eugenie and Eric to schedule your usage of staging. Eugenie can run the testing there.

I should also note that using multiple chained .append() method calls is more efficient than using the + string concatenation operator.

Conversion test passed.

I have applied this to staging but it is NOT yet rebuilt. Later today, some other changes will be applied, the code will be rebuilt and then testing will be run.

Tested the behavior for UNIX 4GL and noticed notable differences than the Windows 4GL:

- The current dir "." is not taken into consideration.
- The upper directory ".." appears as hidden even if it isn't - Bug that we will probably have to replicate
- The hidden directories also appear as hidden compared to windows where only hidden files were marked as hidden
- The order in which the indices appear is different it seems additive types are first followed by the main types as opposed to windows 4GL
Ex: HD, LF , LD, HF, HD, HLF HLD, FP (FP is actually the only exception).

The main part of the file type is the same tough as for FILE-INFO:FILE-TYPE.

OK, I will apply the update to staging so we can retest. If it passes we will check it in, even though there are deviations.

Please prepare a set of fixes for the UNIX cases. Please include careful documentation of the differences in implementation between UNIX and Windows so that when we implement the Windows version of the method, we won't break that code. This new update should only have the minimum changes needed for these fixes AND the changes should be built as a new history entry since it will be checked in separately.

Finished the logic for unix/windows adding another bitfield to the result that will point if unix or windows logic will apply when creating the status string.
While testing I did discover a bug for pipes.
In the filesys.c we use fstat(to get if a link is to directory or file), This requires a file descriptor from opening the file, the problem is when it tries to open a pipe file it will block and freeze (not exactly sure why - i assume if opening a pipe it will expect to get something by someone writing on it).
I'm working on a way to solve this problem at the moment and will add the update as soon as I fix this.

You can't open a pipe like a regular file. It is a very special fake filename that is implemented by code in the kernel that passes information between 2 processes using a kernel buffer. Access is highly serialized and I'm not surprised you block. Is there a different interface you can use that doesn't require you to open it?

The regression testing for cs_upd20130116a.zip has passed, but to clear testing we had to roll back the StreamWrapper change. Please remove that file from the cs_upd20130116a.zip and upload the result as cs_upd20130116b.zip.

Then you can check in cs_upd20130116b.zip to bzr and distribute the change.

Feedback on cs_upd20130117a.zip:

1. The FileSystemDaemon changes are definitely wrong. For example, it is valid for search() to return null if the file/path can't be found. But that will generate an NPE in searchPath. I also don't like the extra unnecessary creation of the File instance. Instead of moving the file restriction to search(), just update searchPath() to include a 4th parameter that is a "boolean allowDir". Then searchPath() will call that with a false and accessFileInfo() would call it with a true. You will also need to fix the search() javadoc, which is wrong already, but which will need additional changes for the new approach.

In search() you would have && (allowDir || file.isFile()) instead of && file.isFile().

2. FileSystemOps should use com.goldencode.util.PlatformHelper.isUnderWindowsFamily() instead of implementing its own Windows OS check.

3. FileChecker.getFileType() should have comments that explain the following:

- The current dir "." is not taken into consideration.
- The upper directory ".." appears as hidden even if it isn't - Bug that we will probably have to replicate
- The hidden directories also appear as hidden compared to windows where only hidden files were marked as hidden
- The order in which the indices appear is different it seems additive types are first followed by the main types as opposed to windows 4GL
Ex: HD, LF , LD, HF, HD, HLF HLD, FP (FP is actually the only exception).

The main part of the file type is the same tough as for FILE-INFO:FILE-TYPE.

4. In FileChecker.getFileType() there are many tests using this idea:

if ((val & IS_WINDOWS) > 0)

Although it is not going to be a problem right now, this approach does have a flaw. A Java int is 32 bits in size and it uses a "twos complement" notation for storage. This means that when the most significant (leftmost) bit is 1 then the number is a negative number.

If we ever use a flag of 1<<31 then the number will be negative and the test above will not properly detect the successful match. Right now this cannot happen. But I see no reason to use a bad practice, when instead the comparison can always be correct. Use this instead:

if ((val & IS_WINDOWS) != 0)

5. BTW, there are still some minor formatting problems in filesys.c. Look for if( instead of if (.

6. Because of the high amount of overlap between this and #1613, you will build a single common update with all the changes from both tasks. You can upload the same file to both tasks so that there is no confusion.

DO NOT WORK ON THIS AT ALL RIGHT NOW. I am putting this here for usage AFTER milestone 4. None of this work is needed until that time.

You don't need separate history entries for #1613 and #1626. Just merge the text into a single entry and use the same update for both.

Two more additions to the feedback:

7. DirStream should use com.goldencode.util.PlatformHelper.isUnderWindowsFamily() instead of implementing its own Windows OS check.

8. Please remove isUnix() from DirStream since it is not used.

Just like above, this feedback is only for after milestone 4 is done. No further work is to be done right now.

Please describe the reasons behind the DirStream.getPathName() change in cs_upd20130404a.zip.

Sorry I forgot to mention about that. The reason is when we have a link, getting the canonicalPath of the link file will return the path to the file/folder the link is pointing to and this resulted in wrong detection.
Instead I've created the path from canonicalPath of the parentFile + the file name.

OK, that is fine. Please put comments into that code to explain this, so that future readers will not accidentally change this to the simpler version.

Update cs_upd20130418b from #1613 includes this. It has passed conversion and runtime regression testing.
Commited to bzr as revision number 10341.