RE: 3440:Operation not supported when trying to read an IFS file. -- RPG400-L

Scott,

Today I read your e-mail in detail and I appreciate the time and effort you 
have put in to help me resolve the problem I was facing with my code. 

I will change my code accordingly.

Again my appreciation to every one for their advice.

Regards

Emmanuel

-----Original Message-----
From: rpg400-l-bounces@xxxxxxxxxxxx
[mailto:rpg400-l-bounces@xxxxxxxxxxxx]On Behalf Of Scott Klement
Sent: 10 November 2005 8:54 PM
To: RPG programming on the AS400 / iSeries
Subject: Re: 3440:Operation not supported when trying to read an IFS
file.



Hi Emmanuel,

I'll give you my opinions on your code.  Some of my suggestions relate to 
actual bugs in your implementation, and others aren't causing problems, 
but just make maintenance more difficult or making the code more 
confusing.  Here are my thoughts:

[SNIP]
> 0005.00  * Open - Open an IFS file
> 0006.00 D open            PR            10I 0 ExtProc('open')
> 0007.00 D  filename                       *   Value
> 0008.00 D  openflags                    10I 0 Value
> 0009.00 D  mode                         10U 0 Value options(*nopass)
> 0010.00 D  codepage                     10U 0 Value options(*nopass)

Unless you need your code to work at V3R2, you should have 
OPTIONS(*STRING) on the filename parameter.  That way, you don't need to 
do the extra work of adding x'00' to the filename, and you don't need to 
pass a pointer to the API.

That will make your code easier to read, and will save you work when 
writing it.

Also, you should have your prototypes, constants and data structures that 
are related to the IFS APIs in a /COPY member. Here's why:

a) You won't have to redefine them every time you write a program, this 
reduces the potential for errors and saves you time.

b) It ensures that the definitions consistent across all programs. That 
way, someone reading your code won't make incorrect assumptions about how 
the prototype works, and won't have to study the prototypes in every 
program.

c) Because you don't list all of the definitions in your source code, 
directly, the code is shorter, simpler and easier to read.


> 0014.00 D access          PR            10I 0 ExtProc('access')
> 0015.00 D  filename                       *   Value
> 0016.00 D  modeI                        10U 0 Value

Options(*STRING) shoudl also be added to the filename here.

> 0034.00 D unlink          PR            10I 0 ExtProc('unlink')
> 0035.00 D  filename                       *   Value

Options(*STRING) should also be added here.


> 0063.00 D CodePage        S             10U 0 Inz(437)

I'd recommend against using 437 as a codepage unless you're still working 
with old copies of MS-DOS.  Instead, consider using 1252 (Windows 
Latin-1).


> 0067.00  * Values for oflag parameter, used by open()
> 0068.00 D O_APPEND        S             10I 0 Inz(256)
> 0069.00 D O_CODEPAGE      S             10I 0 Inz(8388608)
> 0070.00 D O_CREAT         S             10I 0 Inz(8)
> 0071.00 D O_EXCL          S             10I 0 Inz(16)
> 0072.00 D O_RDONLY        S             10I 0 Inz(1)
> 0073.00 D O_RDWR          S             10I 0 Inz(4)
> 0074.00 D O_TEXTDATA      S             10I 0 Inz(16777216)
> 0075.00 D O_TRUNC         S             10I 0 Inz(64)
> 0076.00 D O_WRONLY        S             10I 0 Inz(2)

These should all be constants rather than variables!!  You don't want 
someone to be able to change the value of O_TEXTDATA, for example. You 
want the name O_TEXTDATA to be a synonym for 16777216.

Making these constants instead of variables makes your code more 
self-documenting, because you make it clear that the names (like 
O_TEXTDATA) are symbolic representations of the numbers.

Since you used variables here, the next guy who reads the code could 
potentially think that these are not symbolic representations, but rather 
are variables that control something.

For example, he could think that you want to append to the file 256 times 
and that if he wanted to change the numebr of times, he should change that 
variable.  That would be incorrect thinking, but it's a conclusion you 
could lead him to making these variables instead of constants.

> 0078.00  * User authorities for omode paramter, used by open()
> 0079.00  * from QSYSINC/SYS, member STAT
> 0080.00 D S_IRUSR         S             10I 0 Inz(256)
> 0081.00 D S_IWUSR         S             10I 0 Inz(128)
> 0082.00 D S_IXUSR         S             10I 0 Inz(64)
> 0083.00 D S_IRWXU         S             10I 0 Inz(448)
> 0084.00
> 0085.00  * Group authorities
> 0086.00 D S_IRGRP         S             10I 0 Inz(32)
> 0087.00 D S_IWGRP         S             10I 0 Inz(16)
> 0088.00 D S_IXGRP         S             10I 0 Inz(8)
> 0089.00 D S_IRWXG         S             10I 0 Inz(56)
> 0090.00
> 0091.00  * Other authorities
> 0092.00 D S_IROTH         S             10I 0 Inz(4)
> 0093.00 D S_IWOTH         S             10I 0 Inz(2)
> 0094.00 D S_IXOTH         S             10I 0 Inz(1)
> 0095.00 D S_IRWXO         S             10I 0 Inz(7)

Same problem here.  These should be constants rather than variables!!

> 0101.00 D Null            C                   Const(x'00')

I'd try to avoid using "Null" in your program, but if you do have to use 
it, this is a good way to do it.  I'm happy to see that this one, at 
least, is a constant.


> 0105.00    // Set Pc file details
> 0106.00   L = Pr_SetFile ();
> 0107.00   If L = -1;
> 0108.00
> 0109.00      ExSr *PSSR;
> 0110.00
> 0111.00   EndIf;
> 0112.00
> 0113.00     // Open PC file in IFS directory
> 0114.00   L = Pr_OpnFile ();
> 0115.00   If L = -1;
> 0116.00
> 0117.00      ExSr *PSSR;
> 0118.00
> 0119.00   EndIf;

The reason you call Pr_SetFile() is to create a file with the right 
attributes in the IFS.  Then you call Pr_OpnFile() to open that file and 
turn on ASCII/EBCDIC translation.

The problem is, you never close the file you opened with Pr_SetFile(). 
So, instead of creating it and then opening it, you've resulted in opening 
it twice, in two different modes.


> 0123.00 C                   eval      nbytesread = read(FileDescI:%addr(bufI):
> 0124.00 C                                           nbyteset)
> 0125.00
> 0126.00 C                   If        nbytesread > 0
> 0127.00 C                   Eval      *InLr = *On
> 0128.00 C                   Else
> 0129.00 C                   Callp     DspError
> 0130.00 C                   EndIf


In Pr_OpnFile() you set a variable called Err_Flag.  That's a confusing 
name for what is actually a file descriptor.  This descriptor needs to be 
passed to every API that you want to work with that file.

That's how the operating system keeps track of all of the different files 
you have open.  Each time you open a new one, it assigns it a number. 
Then when you want to operate on that file, you pass the number back to 
the API that operates on it.

In this case, however, you're not doing that.  Err_Flag is the file 
descriptor, but you're passing something else (FileDescI) as a parameter 
to read().  So, the read() won't work.  I assume that this is a copy/paste 
error, since it wouldn't provide the error message that you referred to.


> 0132.00  
> **********************************************************************
> 0133.00  * *Inzsr - Program level initialisation
> 0134.00  
> **********************************************************************
> 0135.00 C     *Inzsr        BegSr
> 0136.00
> 0137.00  * Received Parameters
> 0138.00 C     *Entry        Plist
> 0139.00 C                   Parm                    PrmFileName
> 0140.00 C                   Parm                    PrmPssrErr
> 0141.00
> 0142.00  /free
> 0143.00
> 0144.00   // Default path name
> 0145.00   Filename = %Trim(PrmFileName);
> 0146.00
> 0147.00  /end-free
> 0148.00
> 0149.00 C                   EndSr



In this routine you use %trim() to remove the blanks from PrmFileName. 
Unfortunately, it doesn't work.  The "Filename" field is a fixed-length 80 
character field.  It always has to have 80 characters in it, it can never 
have less.

When you assign something that's less than 80 characters to an 
80-character field, what does it do?  How does it ensure that the field 
has all 80 chars filled it?

It adds blanks.

So, in essence, this is what your %trim() line does:

a) Gets the contents of the PrmFileName variable.

b) Removes all of the blanks from it.

c) Adds blanks back on until the field is 80 chars long.


That's confusing, and it makes you look like you don't know what you're 
doing.  If you want to remove teh blanks and have them stay off, use a 
VARYING field for "filename".

If your goal is to only remove the blanks from the left (which is the net 
result) and not from the right end of the string, then use %TRIML, it'll 
be faster.

Alternately, just use PrmFileName directly on the open() call, and put the 
%trim() right there on the open() statement.  That way, the output without 
the blanks won't ever be assigned to a fixed-length string, instead it'll 
be passed to the API.


> 0171.00  
> **********************************************************************
> 0172.00  * Pr_SetFile - Set PC file details
> 0173.00  
> **********************************************************************
> 0174.00 P Pr_SetFile      B
> 0175.00
> 0176.00 D Pr_SetFile      PI             1  0
> 0177.00
> 0178.00  /free
> 0179.00
> 0180.00   Oflag = O_CREAT + O_CODEPAGE + O_RDWR + O_TRUNC;
> 0181.00
> 0182.00   Omode = S_IRWXU + S_IRWXG + S_IROTH + S_IXOTH;
> 0183.00
> 0184.00   Filename = %Trim(FileName) + Null;
> 0185.00
> 0186.00  /end-free
> 0187.00
> 0188.00 C                   Eval      Err_Flag = Open(%Addr(Filename) : OFLAG 
> :
> 0189.00 C                                        Omode: CodePage)
> 0190.00
> 0191.00  /free
> 0192.00
> 0193.00   If Err_Flag < 0;
> 0194.00      Return -1;
> 0195.00   Else;
> 0196.00      Return 0;
> 0197.00   EndIf;
> 0198.00
> 0199.00  /end-free
> 0200.00
> 0201.00 P Pr_SetFile      E



I don't understand why this routine switches in and out of free format. 
Why not make the open() API call be free-format?  That would be easier to 
read, since you'd have more space.

Also, as I mentioned above:

a) you should be using options(*string) so you don't have to manually add 
NULL, and you don't have to use %addr() with the open() API.

b) you should remember to close the file here.

c) Subprocedures should avoid using global variables if possible, and in 
this case, it would be easy to do so.  Why use globals?  All it does is 
make your code less reusable.



> 0205.00  
> **********************************************************************
> 0206.00  * Pr_OpnFile - Open PC File in IFS directory
> 0207.00  
> **********************************************************************
> 0208.00 P Pr_OpnFile      B
> 0209.00
> 0210.00 D Pr_OpnFile      PI             1  0
> 0211.00
> 0212.00  /free
> 0213.00
> 0214.00   Oflag = O_WRONLY + O_TEXTDATA;
> 0215.00
> 0216.00  /end-free
> 0217.00
> 0218.00 C                   Eval      Err_Flag = Open(%Addr(Filename) : Oflag)
> 0219.00
> 0220.00  /free
> 0221.00
> 0222.00   If Err_Flag < 0;
> 0223.00      Return    -1;
> 0224.00   Else;
> 0225.00      Return 0;
> 0226.00   EndIf;
> 0227.00
> 0228.00  /end-free
> 0229.00
> 0230.00 P Pr_OpnFile      E

Here you've opened the file for WRITING ONLY (O_WRONLY), but the mainline 
tries to use the read() API to read it.

That's why you're getting an ENOTSUPP error.  Because it doesn't support 
reading from a write-only file.


> 0236.00 P DspError        B
> 0237.00
> 0238.00 D DspError        PI
> 0239.00
> 0240.00 D text            S             10A
> 0241.00
> 0242.00 D errorNo@        S               *   Inz
> 0243.00 D  errorNo        S             10I 0 Based(Errorno@)
> 0244.00 D errorMsg@       S               *   Inz
> 0245.00 D  errorMsg       S            100A   Based(errorMsg@)
> 0246.00 D errortxt        S             52A   Inz(*Blanks)
> 0247.00
> 0248.00
> 0249.00  /free
> 0250.00
> 0251.00   errorNo@ = GetErrNo;
> 0252.00
> 0253.00   errormsg@ = StrError(errorNo);
> 0254.00
> 0255.00   errortxt  = %trim(text) + '->' +
> 0256.00                   %char(errorNo) +
> 0257.00                   ':' + %subSt(errormsg:1:37);
> 0258.00  /end-free
> 0259.00
> 0260.00
> 0261.00
> 0262.00 P DspError        E


The string that StrError() points to isn't 100A.  It can be any length 
from zero to hundreds of bytes long.  The way you can tell when you reach 
the end of the string is that it'll have a null (x'00') character.

Unfortunately, this code doesn't take that into consideration.  Instead, 
it just takes the first 100 bytes of the error message (whether the 
message is that long or not!) and reads it.

Then, after making the code look at 100 bytes of memory, even though it 
doesn't make sense, you turn around and chop off the first 37 bytes of 
that 100 bytes.  (Makes me wonder why you used 100A instead of 37A in the 
first place)

The code is confusing and incorrect.

Instead, get rid of the errormsg@ and errormsg fields.  They're 
unnecesary.  Make the routine look like this:

     D errorNo@        S               *   Inz
     D  errorNo        S             10I 0 Based(Errorno@)
     D errortxt        S             52A   Inz(*Blanks)

      /free

         errorNo@ = GetErrNo;

         errortxt = %trim(text) + '->' +
                    %char(errorNo) +
                    ':' + %str(strerror(errorNo));

The %STR() BIF will read the C-style (null-terminated) string from memory 
and convert it to an RPG style one.  I can add the result of %STR() right 
onto my error message.


Anyway, I hope that helps.