|
Post by mueh on May 19, 2023 1:47:13 GMT -5
Hi George ! Crash in CopyAFile when Profile has RECFM=U EOL=NONE and LRECL=80 There are 3 Concern's
1. Crash which doesn't occure with RECFM=F (But it does Blank Padding so it's unusable for modifying Binary data ) 2. Blank padded msg should not occure for RECFM U and should not be done . 3. Crash Report doesn't show a Module Back Trace in newer Beta versions .
Following code change seems to cause the problem Solved it by using a XFORM macro . To reproduce edit a file with above profile parameters . Thanks
00001 ----- '--------------------------------------------------------------------------------------+
00002 ----- '- A simple delimited line V3.0.23088 |
00003 ----- '--------------------------------------------------------------------------------------+
00004 ----- ELSEIF ZPrf.RECFM = "U" THEN ' If a delimited type file
00005 ----- i = INSTR(CP, Chunk, dlm) ' See if there's a line present
00006 ----- IF i THEN ' If there is
00007 ----- TD.TFrom = CP: TD.TLen = i - CP: TD.TPage = 0 ' Setup for InsertT
00008 ----- CP = i + LEN(dlm) ' Step over
00009 ----- GOSUB InsertT ' Go insert the line
00010 ----- ELSE ' Remainder of buffer
00011 ----- TD.TFrom = CP: TD.TLen = bytesinfile - CP + 1: TD.TPage = 0' Setup for InsertT
00012 ----- GOSUB InsertT ' Go insert the line
00013 ----- CP = bytesinfile + 1 ' Exit
00014 ----- END IF '
+++++ 00001 '----- A simple delimited line V2.0.19350
+++++ 00002 ELSEIF ZPrf.RECFM = "U" THEN ' If a delimited type file
+++++ 00003 IF INSTR(Chunk, dlm) OR _ ' If there's a line present
+++++ 00004 bytesleft = 0 THEN ' Or we're simply out of data
+++++ 00005 t = EXTRACT$(Chunk, dlm) ' Read the data
+++++ 00006 Chunk = REMAIN$(Chunk, dlm) '
+++++ 00007 GOSUB InsertT ' Go insert the line
+++++ 00008 ELSE ' We need continued data
+++++ 00009 leftover = Chunk ' Save whats left
+++++ 00010 EXIT DO ' Exit to read more data
+++++ 00011 END IF '
|
|
|
Post by George on May 19, 2023 9:58:12 GMT -5
MUEH: The error here isn't the crash, the error is that you are not supposed to be able to create a set of parameters that's illogical. I'm surprised you managed to do it, it took me several tries to avoid having the commands rejected. LRECL is not supposed to be allowed with RECFM=U.
So what format IS this file in? How SHOULD it be deblocked into records?
The EOL command seems to be the culprit that allows it to get through.
The differences in CopyAFile between versions was part of the changes quite a while ago to improve the loading of huge files (e.g. 1,000,000+ lines) The code is an innocent victim of these illogical parameters.
I'll look at the back trace weirdness.
George
|
|
|
Post by George on May 19, 2023 10:25:19 GMT -5
MUEH: Very difficult code area. There are warnings added to some of the RECFM/LRECL/EOL messages. If the warnings are ignored, stuff like this slips through. There really should be one command that sets all 3, so proper validation can be done. Some kind of command like FORMAT U CRLF 0 or FORMAT F NONE 80 to set the RECFM/EOL/LRECL in one command.
George
Basically the rules are:
Illogical combinations
RECFM=U Cannot be with EOL=NONE or LRECL>0
RECFM=F Cannot be with LRECL=0
RECFM=V Cannot be with EOL¬=NONE or LRECL>0
EOL=NONE Cannot be with RECFM=U
EOL¬=NONE Cannot be with RECFM=V
LRECL=0 Cannot be with RECFM=F
LRECL>0 Cannot be with RECFM=U or RECFM=V
I was going to review the RECFM/EOL/LRECL commands try and close the holes, but it won't work. So I think the best approach is a new single command to specify all three values in one command.
So I think it will be FORMAT recfm eol lrecl.
Comments appreciated.
George
|
|
|
Post by mueh on May 20, 2023 1:37:41 GMT -5
George: The idea of using RECFM=U with LRECL was to chop the file into Line's and after modifying hoping that SPFLite writes the lines back as one string or use a Macro to write it as one string . I don't want to make you more work than needed . Maybe DOC change under RECFM U
U The file uses Undefined type records. i.e. The records are separated by unique delimiters which should be specified using the EOL command. and failing the File Load in CopyAFile if RECFM=U and dlm="" is enough . It's up to you how you want to solve the crash . I'm now using an XFORM macro to do the work in a few lines of code . Thanks
|
|
|
Post by Stefan on May 20, 2023 4:42:26 GMT -5
Hi George, Re Comments...
I reckon anyone who is familiar with terms like RECFM, LRECL, etc is familiar with the term DCB.
Maybe that's a suitable 'new command' which would retain a relationship to SPFLite's ancestor and the z/OS users. If you do create a new command combining these values, you should probably also revise the presentaion in the FM - CONFIG tab when editing profiles' "File Data Options".
|
|
|
Post by George on May 20, 2023 8:12:34 GMT -5
Stefan: Yes, maybe DCB is more familiar, I wasn't really happy with FORMAT. I'll have a look at the CONFIG display, it was already on the list.
MUEH: The code change was small, but as always the Doc. is more work.
George
[UPDATE]
I'm posting a new Beta. RECFM, EOL, and LRECL commands are gone, replaced by the new DCB command
DCB record-format end-of-line record length
e.g. A standard TXT file's DCB would be DCB U CRLF 0
[/UPDATE]
|
|
|
Post by Robert on May 20, 2023 12:10:15 GMT -5
I hesitate to offer advice of any kind, but I feel the DCB idea is a little cryptic. In your example, DCB U CRLF 0 is (I presume) the same as RECFM U EOL CRLF LRECL 0.
Why not just let users enter them like THAT?
How? Well, yes, it would require more syntax handling, but suppose you were to 'enhance' EACH of the commands RECFM, EOL and LRECL, so that they would ALL take ALL of the other keywords as 'extensions'?
So, RECFM would take EOL and LRECL as keyword operands, EOL would take RECFM and LRECL, and LRECL would take RECFM and EOL.
That way, no one has to learn a new keyword (DCB) and ALL of the prior syntax is still valid - EXCEPT that you would (possibly) have to enter all of them at the same time.
As a reminder, we have SPFLite users who are NOT ex-IBM users, and just might not know what the heck a DCB is.
As a safeguard, you *could* assume that any 'extra' keywords would default to some safe (and valid) values.
For instance, a standard TXT file is RECFM U EOL CRLF LRECL 0.
So, if I say RECFM U under the 'new' way of doing things, the EOL CRLF LRECL 0 gets inserted by default.
Does that mean the DCB idea should be dropped? No, not necessarily.
Instead, suppose we let DCB mean something new: A *NAMED* set of file format attributes.
Example: To get the default settings for a text file, you could do this:
DBC TXT
Isn't that nice?
Then, where do those defaults COME from, anyway? From SET symbols.
Like this: SET DCB.TXT = "RECFM U LRECL 0 EOL CRLF"
Let us assume that DBC is a new (optional) PROFILE setting (it is NONE when not set). Then, when a profile has its DCB set to a set-name, the other values are set to DBC automatically. That means they are dependent on the DBC SET name.
Example: Let's say the PROFILE for files of type TXT has these settings, where the DCB keyword has a value, and the other 3 have an "off-like" value of DCB. It's easier to show than to explain it:
PROFILE TXT, DCB TXT, RECFM DCB, LRECL DCB, EOL DCB
Then, whenever a file with a DCB TXT is opened, SPFLite looks up the DCB.TXT set values. These values are fetched from the SET table each time the file is opened.
A value of EOL DCB would mean, "use the DBC set symbols for the EOL setting". To avoid confusion/complication, you could do this differently. Suppose EOL is CRLF because the DCB.xxx symbol says so. Then, you could show "eol crlf" instead of "EOL CRLF" and have the lower-case in the PROFILE display mean "this value was obtained from the DBC.xxx value.
To make this work, any time a DCB.xxx set value is entered or updated, SPFLite would *validate* the values, and not merely store them. That way, users would be prevented from creating a bad DCB set string. That means the SET primary command and SET Edit would have to do a post validation of the string whenever the name was DCB.xxx.
Finally, SPFLite should create a built-in DBC.TXT set name, at least, since everybody would need it. I am uncertain if any OTHER built-in DCB.xxx names would be needed. Maybe someone else has thoughts on that.
Well, that's the idea.
I will step aside and let you discuss this, without further involvement from me.
|
|
|
Post by George on May 20, 2023 14:19:26 GMT -5
Robert: Your idea has lots of merit, but a) it buries 'where the value came from' to a high degree. and b) it requires much more extensive changes. e.g. we would now have 4 commands, all of which have to parse and validate all the parameters (RECFM, LRECL, EOL and DCB) as well as having the SET command perform all the same stuff to validate the DCB value settings. and c) loading Profile values would have to now cope with having DCB value substitution handled.
As I've said, I'm not prepared anymore to tackle such extensive modifications. And right now, if I don't feel like it, it doesn't happen. The days of grand schemes are done.
George
|
|
|
Post by Stefan on May 21, 2023 6:24:30 GMT -5
Hi George, Thoughts regarding usage and functionality of the new DCB command... (1) I see DCB takes 3 'value' operands, equivalent to the values accepted by the retired RECFM, EOL and LRECL commands, and they must be spoecified in that order. I think this isn't good practice as it doesn't follow the "operands specification" convention for other SPFLite commands.
Given that the old EOL/RECFM/LRECL commands have no overlapping keywords, I think the DCB command should recognise ALL their operands as keywords/keyvalues. Once DCB operands are treated like 'keywords/keyvalues', they can be entered in any order, as opposed to being positional. (In my simplistic mind, you could just reuse the existing operand capture code for the old comands BUT modify the hexadecial entry of the old EOL command to be specified like X'hh' / x'hhhh' to avoid confusion with a decimal LRECL number. And oldies like me/us can continue to think in terms of RECFM followed by LRECL and type DCB F 80 CRLF)
(2) It might(?) also be preferable (less error-prone), if users could just specify the operand or operands they wish to change, rather than entering all three every time.
The effect would be to only modify the value(s) the user enters, after apporpriate cross-validation with the unchanged profile values, of course.
(3) I see DCB ? works as usual.
As yet there is no Get_Profile$("DCB") support, but I guess you'll add that in due course to return just the three values (in whatever order you prefer) I guess you'll have to retain the Get_Profile$(...) support for the existing RECFM, LRECL and EOL commands to ensure no macros get broken, and mark them as "deprecated - replaced by DCB" in the documentation. This may also apply to the older 3 commands, but I doubt they are issued via SPF_CMD(...) from within macros.
|
|
|
Post by George on May 21, 2023 8:19:12 GMT -5
Stefan: Yes, I guess order independence and optionality would be more in keeping with SPF syntax. I'll have a go at switching, I really wasn't happy either with the forced order.
Yes, Get_Profile$ will still respond to RECFM LRECL and EOL. DCB is not added yet.
George
|
|
|
Post by mueh on May 21, 2023 8:28:57 GMT -5
George : With RECFM=F any EOL other than NONE is illogical and should be rejected . It causes remove of characters from record ( except for first ) by length of DLM . Thanks Attached the EBCDIC test file IBMUSER.MU.CVAFEXT (845 B)
|
|
|
Post by George on May 21, 2023 12:09:28 GMT -5
MUEH: Had to go back and correct some very old code here which got mucked up by various optimizations over the years. Shows how often it has been used. RECFM=F files CAN have an EOL specified. If they do, the EOL is stripped off on reading and replaced on writing. So for your example test file RECFM=F EOL=NONE LRECL=65 is correct. If EOL is switched to CRLF the data is written as 65 bytes + CRLF BTW - you should have PRESERVE ON and XTABS 0 since it contains binary data Here's your test file back with CRLF added, I'll post another Beta with the repaired old code in it. Previous Beta won't handle it correctly. I'll add an 'A' to the corrected version. George IBMUSER.MU.CVAFEXTCRLF (871 B)
|
|
|
Post by Robert on May 22, 2023 19:55:21 GMT -5
I wish that this issue could have been presented as a proposal and discussed by the user community first, instead of rushing in and 'solving' the problem first and afterwards waiting for the usual debates and haggles. The reason I presented what I did as an IDEA is that ideas are supposed to spark the imagination, instead of layout down rigid specifications which themselves lead to arguments over details. That is why I didn't add any further points, to give you time to work it out yourselves. I am disappointed in what I see so far. I fully know your interest in minimizing any changes, whereas you surely know that I prefer to see changes implemented as software and not simply as code fixes.
That being said, I believe there is a straight forward way of resolving this that will NOT require any new commands OR new command syntax, which will NOT end up as a butcher job and WON'T involve changing the structure of the profile data.
(Now, I *do* think that using DCB.xxx SET names was a good idea, but I know that I no longer have standing here, so I won't pursue that.)
Here is how it could be done:
1. Leave ALL of the existing RECFM, LRECL and EOL commands as is. (And, if it turns out that any *additional* commands could interact and conflict in similar ways, they TOO would remain as-is.)
2. For ALL of the commands that could possibly interact and create conflicts, you need to exhaustively list every meaningful combination of essential values. Examples:
(a) For LRECL, 'essential' values are 0 and > 0 (b) For RECFM and EOL, these are just lists of the possible valid values. (c) If any other commands or settings end up as part of this issue, you'd add them to the list.
3. Once the list in (2) is developed, resolve which combinations of values are OK and which are not.
4. The valid-combination list should be publicized (that is, put in the Help) so people know what is and isn't allowed.
5. The list in question should be a table in memory, so that if the rules needed to be changed in the future, you just change and interpret that table instead of changing logic.
6. You would create some kind of profile-interaction-validation function (hard to come up with a good name here). Let's call it Cross-Validate() for short.
7. For every command that could change a profile setting in a way that could cause a conflict, after saving the changed value, you would call this Cross-Validate() function, which makes sure there is no combination of settings that conflict with each other. By creating this function, it basically would amount to a one-line change to each of the functions that handle the Primary commands of RECFM, LRECL, EOL and others that might end up needing to be cross-validated.
8. If Cross-Validate() finds an error, it would (probably) issue an error message explaining what is wrong, and then bring up the (existing) Profile Edit GUI. Once in the GUI, any errors that need correction could be done right then and there.
9. Cross-Validate() would also be called by the Profile Edit GUI itself afterwards, so that if the values were changed by the GUI first instead of by Primary commands, they would still get caught.
---
Net result: No major structural changes to the code, no new commands like DCB, no syntax changes to existing commands, no impact to the profile data, and no invalidation of existing macro code, if any exists. Only possible impact is if a macro issues something like:
SPF_CMD("RECFM F")
the macro logic would need to check the return code to make sure it was deemed OK by the underlying Cross-Validate() call.
P.S. The MVS concept of a DCB, where file attributes are obtained from an external source, is most closely matched by the suggestion to have a DCB.xxx SET value. But, you didn't want that. If you have no interest in actually creating new functionality that resembles a real MVS DCB, I'd say drop the name altogether. It's confusing for mainframe people who might expect a real DCB and won't get one, and non-mainframe people don't know or care what a DCB means anyway. I was an MVS systems program, and I personally barely remember what it means, and I think you would struggle to explain or justify it in the SPFLite world.
|
|
|
Post by George on May 23, 2023 9:55:12 GMT -5
Robert:
The underlying problem here is that the old RECFM LRECL and EOL commands already DID the needed validation and detected the errors, but only issued a warning message. It was left to the user to make another change to correct the incompatibility. And if they didn't, we are left with an illogical combination. Which is exactly what triggered the crash that MUEH reported that started this topic.
Why only a warning? Because it took multiple commands to complete a change. If it had balked and kicked out the change, we'd end up in a deadlock.
Picture changing from RECFM=U LRECL=0 EOL=CRLF to RECFM=F LRECL=80 EOL=NONE The RECFM would fail because LRECL was zero The LRECL would fail because RECFM was U
Your table based validation would do nothing to alleviate this problem. Only a command that supports changing all 3 simultaneously can effectively perform validation. Yes, this can be currently done in the Profile GUI, but that leaves no way to do it on the command line
Hence the DCB command, which was a simple combining of 3 commands into one, there was no change made to the Profile data structure.
But there is a way around this. Eliminate DCB as a command name, and re-institute the RECFM, LRECL and EOL commands. Except all three commands would now point at the combined DCB code, which handles all the operands of the three old commands.
This means that any of the 3 commands can be used to set any desired combinations. It also means proper validation is still done. e.g. EOL NONE F 80 or RECFM V 0 NONE or LRECL 100
This means no impact on any existing command or macro usage. It just makes describing it in the Doc. a little weird.
I think this would solve the problem.
George
|
|
|
Post by Robert on May 23, 2023 13:41:58 GMT -5
You have identified the underlying problem in the first two sentences. By issuing a warning, but not insisting on a correction, you are allowing the very inconsistency that is the cause of the current problem.
Without debating command syntax (which doesn't really need to be changed in the first place), I believe the right solution needs to be handled along these lines:
1. Let us say that the "DCB concept" is not flawed per se, but it needs to be redefined. Suppose there is a key set of file attributes that MUST be mutually consistent, or else the "DCB part" of a PROFILE is invalid. Right now, this set of "DCB attributes" is being presented as the three values RECFM, LRECL and EOL. It is *possible* you might want to ALSO include MINLEN, BOM and SOURCE. I don't want to go into details on this, except to say that as time goes on, you might see the need to *expand* the idea of what needs to be considered in this "set" of values that must not conflict with each other. So, the list of "DCB attributes" might eventually end up with more that 3 values.
2. Let us further say that you will NOT ALLOW a profile to be created or changed with inconsistent "DCB values". For example, with EOL NONE, you have to have some way of defining a record. You'd either need RECFM V or LRECL n where 'n' is > 0. Whatever the rules are, you would simply not be allowed to make a bad PROFILE. If the user tried, you would send them to the PROFILE edit GUI and *insist* that they fix it. The only thing they would be allowed to do is either correct the problem, or cancel the profile edit screen. A "cancel" would level could leave the profile "broken", but you couldn't use it to edit anything. So, you may or may not want to even allow a Cancel option. Instead, you could either force some set of defaults, or perhaps include a new "flag" in the profile to indicate it was "damaged or incomplete" so you could defer correction to later, but you can't edit with a damaged or incomplete profile. Inclusion of that flag is a separate issue. If you went that way, perhaps all existing profiles would be marked as "unverified" so you'd have to verify every profile at least once. (Or, the "flag" might simply be the results of the Cross-Verify return code without actually changing the SQLite attributes of the Profile.
3. Some primary commands might inherently create an inconsistency, like EOL NONE by itself. That is OK. What you would do then is (a) accept the NONE setting *provisionally*, but then (b) immediately jump into PROFILE edit. That way, they users don't wouldn't have to re-specify the EOL part, but they would be forced to add or change other values, as needed, to make the whole thing consistent.
4. One hitch to this is that if (somehow) the profile were ALREADY invalid, you'd be in a bind. The only way that could happen is if there were defined a profile that was "inconsistent" according to any (new?) rules you defined today, but had been saved in the past, when those rules had not yet been enforced. If that problem existed, then either at the next file-edit time, or at PROFILE edit time if you were to directly edit a profile, you would have to be prepared to immediately jump into "Profile Repair Mode".
The main point is that if you force a profile to be "valid" and refuse to allow a profile ever to be created or modified in an invalid state, it should resolve the underlying problem.
|
|